From cd423672090cfb55e0eb3f010e88db8b5c1a225f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20Manuel=20Mari=C3=B1as=20Bascoy?= Date: Wed, 21 Jun 2023 14:50:21 +0200 Subject: [PATCH 01/31] improve getMerggedAbi function filtering --- packages/contracts/utils/abi.ts | 43 ++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/packages/contracts/utils/abi.ts b/packages/contracts/utils/abi.ts index c5486f32f..825fb9a24 100644 --- a/packages/contracts/utils/abi.ts +++ b/packages/contracts/utils/abi.ts @@ -1,4 +1,4 @@ -import {HardhatRuntimeEnvironment} from 'hardhat/types'; +import {Artifact, HardhatRuntimeEnvironment} from 'hardhat/types'; export async function getMergedABI( hre: HardhatRuntimeEnvironment, @@ -6,23 +6,42 @@ export async function getMergedABI( secondaryABIs: string[] ): Promise<{abi: any; bytecode: any}> { const primaryArtifact = await hre.artifacts.readArtifact(primaryABI); - - const secondariesArtifacts = secondaryABIs.map( - async name => await hre.artifacts.readArtifact(name) + // read all secondary artifacts + const secondariesArtifacts = await Promise.all( + secondaryABIs.map((abi: string) => hre.artifacts.readArtifact(abi)) ); - const _merged = [...primaryArtifact.abi]; - for (let i = 0; i < secondariesArtifacts.length; i++) { - const artifact = await secondariesArtifacts[i]; - _merged.push(...artifact.abi.filter((f: any) => f.type === 'event')); + // filter events from secondaries artifacts + for (const artifact of secondariesArtifacts) { + _merged.push(...artifact.abi.filter(f => f.type === 'event')); } // remove duplicated events - const merged = _merged.filter( - (value, index, self) => - index === self.findIndex(event => event.name === value.name) - ); + const merged = _merged.filter((value, index, self) => { + // filter events that meet the following conditions: + // - have the same name + // - have the same number of inputs + // - every input of both events have the same type and name + return ( + index === + self.findIndex(event => { + return ( + // check both events have the same name + event.name === value.name && + // chgeck both events have the same number of inputs + event.inputs.length === value.inputs.length && + // check every input of both events have the same type and name + event.inputs.every((input: any, i: number) => { + return ( + input.type === value.inputs[i].type && + input.name === value.inputs[i].name + ); + }) + ); + }) + ); + }); return { abi: merged, From 0c32df0400cfe9a3e4becd454db9e56c2de3eb9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20Manuel=20Mari=C3=B1as=20Bascoy?= Date: Fri, 23 Jun 2023 10:48:12 +0200 Subject: [PATCH 02/31] remove getmergedAbi usage --- packages/contracts/deploy/helpers.ts | 17 +++---- .../test/framework/dao/dao-factory.ts | 18 +------ .../framework/plugin/plugin-repo-factory.ts | 18 +------ .../test/plugins/governance/admin/admin.ts | 18 +------ .../addresslist/addresslist-voting.ts | 19 +------ .../majority-voting/token/token-voting.ts | 19 +------ .../plugins/governance/multisig/multisig.ts | 19 +------ .../test/test-utils/plugin-setup-processor.ts | 23 +++------ packages/contracts/test/test-utils/repo.ts | 19 ++----- packages/contracts/utils/abi.ts | 50 ------------------- 10 files changed, 27 insertions(+), 193 deletions(-) delete mode 100644 packages/contracts/utils/abi.ts diff --git a/packages/contracts/deploy/helpers.ts b/packages/contracts/deploy/helpers.ts index 3c5f9884a..ed6c0281d 100644 --- a/packages/contracts/deploy/helpers.ts +++ b/packages/contracts/deploy/helpers.ts @@ -5,10 +5,13 @@ import {HardhatRuntimeEnvironment} from 'hardhat/types'; import IPFS from 'ipfs-http-client'; import {findEvent} from '../utils/event'; -import {getMergedABI} from '../utils/abi'; import {Operation} from '../utils/types'; import {VersionTag} from '../test/test-utils/psp/types'; -import {ENSRegistry__factory, PluginRepo__factory} from '../typechain'; +import { + ENSRegistry__factory, + PluginRepoFactory__factory, + PluginRepo__factory, +} from '../typechain'; import {VersionCreatedEvent} from '../typechain/PluginRepo'; import {PluginRepoRegisteredEvent} from '../typechain/PluginRepoRegistry'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; @@ -208,15 +211,7 @@ export async function createPluginRepo( hre ); - const {abi, bytecode} = await getMergedABI(hre, 'PluginRepoFactory', [ - 'PluginRepoRegistry', - ]); - - const pluginRepoFactoryFactory = new ethers.ContractFactory( - abi, - bytecode, - signers[0] - ); + const pluginRepoFactoryFactory = new PluginRepoFactory__factory(signers[0]); const pluginRepoFactoryContract = pluginRepoFactoryFactory.attach( pluginRepoFactoryAddress ); diff --git a/packages/contracts/test/framework/dao/dao-factory.ts b/packages/contracts/test/framework/dao/dao-factory.ts index a0663e0e9..e7367f688 100644 --- a/packages/contracts/test/framework/dao/dao-factory.ts +++ b/packages/contracts/test/framework/dao/dao-factory.ts @@ -37,7 +37,6 @@ import { import adminMetadata from '../../../src/plugins/governance/admin/build-metadata.json'; import {findEvent} from '../../../utils/event'; -import {getMergedABI} from '../../../utils/abi'; import {daoExampleURI, deployNewDAO} from '../../test-utils/dao'; import {deployWithProxy} from '../../test-utils/proxy'; import {getAppliedSetupId} from '../../test-utils/psp/hash-helpers'; @@ -163,22 +162,9 @@ describe('DAOFactory: ', function () { let signers: SignerWithAddress[]; let ownerAddress: string; - let mergedABI: any; - let daoFactoryBytecode: any; - before(async () => { signers = await ethers.getSigners(); ownerAddress = await signers[0].getAddress(); - - const {abi, bytecode} = await getMergedABI( - // @ts-ignore - hre, - 'DAOFactory', - ['DAORegistry', 'PluginSetupProcessor', 'src/core/dao/DAO.sol:DAO'] - ); - - mergedABI = abi; - daoFactoryBytecode = bytecode; }); beforeEach(async function () { @@ -217,9 +203,7 @@ describe('DAOFactory: ', function () { ); // Deploy DAO Factory - const DAOFactory = new ethers.ContractFactory( - mergedABI, - daoFactoryBytecode, + const DAOFactory = new DAOFactory__factory( signers[0] ) as DAOFactory__factory; daoFactory = await DAOFactory.deploy(daoRegistry.address, psp.address); diff --git a/packages/contracts/test/framework/plugin/plugin-repo-factory.ts b/packages/contracts/test/framework/plugin/plugin-repo-factory.ts index 7ee96d196..27afbaefa 100644 --- a/packages/contracts/test/framework/plugin/plugin-repo-factory.ts +++ b/packages/contracts/test/framework/plugin/plugin-repo-factory.ts @@ -18,7 +18,6 @@ import { IProtocolVersion__factory, IERC165__factory, } from '../../../typechain'; -import {getMergedABI} from '../../../utils/abi'; import {getInterfaceID} from '../../test-utils/interfaces'; import {CURRENT_PROTOCOL_VERSION} from '../../test-utils/protocol-version'; @@ -53,22 +52,9 @@ describe('PluginRepoFactory: ', function () { let managingDao: DAO; let pluginRepoFactory: PluginRepoFactory; - let mergedABI: any; - let pluginRepoFactoryBytecode: any; - before(async () => { signers = await ethers.getSigners(); ownerAddress = await signers[0].getAddress(); - - const {abi, bytecode} = await getMergedABI( - // @ts-ignore - hre, - 'PluginRepoFactory', - ['PluginRepoRegistry'] - ); - - mergedABI = abi; - pluginRepoFactoryBytecode = bytecode; }); beforeEach(async function () { @@ -90,9 +76,7 @@ describe('PluginRepoFactory: ', function () { ); // deploy PluginRepoFactory - const PluginRepoFactory = new ethers.ContractFactory( - mergedABI, - pluginRepoFactoryBytecode, + const PluginRepoFactory = new PluginRepoFactory__factory( signers[0] ) as PluginRepoFactory__factory; diff --git a/packages/contracts/test/plugins/governance/admin/admin.ts b/packages/contracts/test/plugins/governance/admin/admin.ts index 411b1d872..6c56ad73d 100644 --- a/packages/contracts/test/plugins/governance/admin/admin.ts +++ b/packages/contracts/test/plugins/governance/admin/admin.ts @@ -2,7 +2,6 @@ import {expect} from 'chai'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {ethers} from 'hardhat'; -import {getMergedABI} from '../../../../utils/abi'; import { findEvent, DAO_EVENTS, @@ -16,6 +15,7 @@ import {toBytes32} from '../../../test-utils/voting'; import { AdminCloneFactory, AdminCloneFactory__factory, + Admin__factory, IERC165Upgradeable__factory, IMembership__factory, IPlugin__factory, @@ -44,20 +44,10 @@ describe('Admin', function () { let dummyActions: any; let dummyMetadata: string; - let mergedAbi: any; - let adminFactoryBytecode: any; - before(async () => { signers = await ethers.getSigners(); ownerAddress = await signers[0].getAddress(); - ({abi: mergedAbi, bytecode: adminFactoryBytecode} = await getMergedABI( - // @ts-ignore - hre, - 'Admin', - ['src/core/dao/DAO.sol:DAO'] - )); - dummyActions = [ { to: ownerAddress, @@ -76,11 +66,7 @@ describe('Admin', function () { }); beforeEach(async () => { - const AdminFactory = new ethers.ContractFactory( - mergedAbi, - adminFactoryBytecode, - signers[0] - ); + const AdminFactory = new Admin__factory(signers[0]); const nonce = await ethers.provider.getTransactionCount( adminCloneFactory.address diff --git a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts index 01442f4d9..155f722ff 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts @@ -4,6 +4,7 @@ import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import { AddresslistVoting, + AddresslistVoting__factory, Addresslist__factory, DAO, IERC165Upgradeable__factory, @@ -19,7 +20,6 @@ import { PROPOSAL_EVENTS, MEMBERSHIP_EVENTS, } from '../../../../../utils/event'; -import {getMergedABI} from '../../../../../utils/abi'; import { VoteOption, pctToRatio, @@ -65,20 +65,9 @@ describe('AddresslistVoting', function () { const startOffset = 10; const id = 0; - let mergedAbi: any; - let addresslistVotingFactoryBytecode: any; - before(async () => { signers = await ethers.getSigners(); - ({abi: mergedAbi, bytecode: addresslistVotingFactoryBytecode} = - await getMergedABI( - // @ts-ignore - hre, - 'AddresslistVoting', - ['src/core/dao/DAO.sol:DAO'] - )); - dummyActions = [ { to: signers[0].address, @@ -102,11 +91,7 @@ describe('AddresslistVoting', function () { minProposerVotingPower: 0, }; - const AddresslistVotingFactory = new ethers.ContractFactory( - mergedAbi, - addresslistVotingFactoryBytecode, - signers[0] - ); + const AddresslistVotingFactory = new AddresslistVoting__factory(signers[0]); voting = await deployWithProxy(AddresslistVotingFactory); diff --git a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts index 0031887e4..55e75c168 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts @@ -13,6 +13,7 @@ import { IPlugin__factory, IProposal__factory, TokenVoting, + TokenVoting__factory, } from '../../../../../typechain'; import { findEvent, @@ -21,7 +22,6 @@ import { PROPOSAL_EVENTS, MEMBERSHIP_EVENTS, } from '../../../../../utils/event'; -import {getMergedABI} from '../../../../../utils/abi'; import { VoteOption, pctToRatio, @@ -69,20 +69,9 @@ describe('TokenVoting', function () { const startOffset = 20; const id = 0; - let mergedAbi: any; - let tokenVotingFactoryBytecode: any; - before(async () => { signers = await ethers.getSigners(); - ({abi: mergedAbi, bytecode: tokenVotingFactoryBytecode} = - await getMergedABI( - // @ts-ignore - hre, - 'TokenVoting', - ['src/core/dao/DAO.sol:DAO'] - )); - dummyActions = [ { to: signers[0].address, @@ -118,11 +107,7 @@ describe('TokenVoting', function () { } ); - const TokenVotingFactory = new ethers.ContractFactory( - mergedAbi, - tokenVotingFactoryBytecode, - signers[0] - ); + const TokenVotingFactory = new TokenVoting__factory(signers[0]); voting = await deployWithProxy(TokenVotingFactory); diff --git a/packages/contracts/test/plugins/governance/multisig/multisig.ts b/packages/contracts/test/plugins/governance/multisig/multisig.ts index ff4ab8d7a..030621978 100644 --- a/packages/contracts/test/plugins/governance/multisig/multisig.ts +++ b/packages/contracts/test/plugins/governance/multisig/multisig.ts @@ -12,6 +12,7 @@ import { IPlugin__factory, IProposal__factory, Multisig, + Multisig__factory, } from '../../../../typechain'; import { findEvent, @@ -20,7 +21,6 @@ import { MULTISIG_EVENTS, MEMBERSHIP_EVENTS, } from '../../../../utils/event'; -import {getMergedABI} from '../../../../utils/abi'; import {deployNewDAO} from '../../../test-utils/dao'; import {OZ_ERRORS} from '../../../test-utils/error'; import { @@ -76,19 +76,8 @@ describe('Multisig', function () { const id = 0; - let mergedAbi: any; - let multisigFactoryBytecode: any; - before(async () => { signers = await ethers.getSigners(); - - ({abi: mergedAbi, bytecode: multisigFactoryBytecode} = await getMergedABI( - // @ts-ignore - hre, - 'Multisig', - ['src/core/dao/DAO.sol:DAO'] - )); - dummyActions = [ { to: signers[0].address, @@ -109,11 +98,7 @@ describe('Multisig', function () { onlyListed: true, }; - const MultisigFactory = new ethers.ContractFactory( - mergedAbi, - multisigFactoryBytecode, - signers[0] - ); + const MultisigFactory = new Multisig__factory(signers[0]); multisig = await deployWithProxy(MultisigFactory); dao.grant( diff --git a/packages/contracts/test/test-utils/plugin-setup-processor.ts b/packages/contracts/test/test-utils/plugin-setup-processor.ts index 1739f9d8f..bf139ac17 100644 --- a/packages/contracts/test/test-utils/plugin-setup-processor.ts +++ b/packages/contracts/test/test-utils/plugin-setup-processor.ts @@ -1,30 +1,21 @@ import {ethers} from 'hardhat'; -import {PluginRepoRegistry, PluginSetupProcessor} from '../../../typechain'; - -import {getMergedABI} from '../../utils/abi'; +import { + PluginSetupProcessor__factory, + PluginRepoRegistry, + PluginSetupProcessor, +} from '../../typechain'; export async function deployPluginSetupProcessor( pluginRepoRegistry: PluginRepoRegistry ): Promise { let psp: PluginSetupProcessor; - const {abi, bytecode} = await getMergedABI( - // @ts-ignore - hre, - 'PluginSetupProcessor', - ['ERC1967Upgrade'] - ); - - const PluginSetupProcessor = new ethers.ContractFactory( - abi, - bytecode, + const PluginSetupProcessor = new PluginSetupProcessor__factory( (await ethers.getSigners())[0] ); - psp = (await PluginSetupProcessor.deploy( - pluginRepoRegistry.address - )) as PluginSetupProcessor; + psp = await PluginSetupProcessor.deploy(pluginRepoRegistry.address); return psp; } diff --git a/packages/contracts/test/test-utils/repo.ts b/packages/contracts/test/test-utils/repo.ts index 17eff06bc..e024e2872 100644 --- a/packages/contracts/test/test-utils/repo.ts +++ b/packages/contracts/test/test-utils/repo.ts @@ -8,9 +8,9 @@ import { PluginRepo__factory, PluginUUPSUpgradeableSetupV1Mock__factory, PluginRepoRegistry__factory, + PluginRepoFactory__factory, } from '../../typechain'; import {deployWithProxy} from './proxy'; -import {getMergedABI} from '../../utils/abi'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; export async function deployMockPluginSetup( @@ -36,23 +36,12 @@ export async function deployPluginRepoFactory( signers: any, pluginRepoRegistry: PluginRepoRegistry ): Promise { - const {abi, bytecode} = await getMergedABI( - // @ts-ignore - hre, - 'PluginRepoFactory', - ['PluginRepoRegistry'] - ); - // PluginRepoFactory - const PluginRepoFactory = new ethers.ContractFactory( - abi, - bytecode, - signers[0] - ); + const PluginRepoFactory = new PluginRepoFactory__factory(signers[0]); - const pluginRepoFactory = (await PluginRepoFactory.deploy( + const pluginRepoFactory = await PluginRepoFactory.deploy( pluginRepoRegistry.address - )) as PluginRepoFactory; + ); return pluginRepoFactory; } diff --git a/packages/contracts/utils/abi.ts b/packages/contracts/utils/abi.ts deleted file mode 100644 index 825fb9a24..000000000 --- a/packages/contracts/utils/abi.ts +++ /dev/null @@ -1,50 +0,0 @@ -import {Artifact, HardhatRuntimeEnvironment} from 'hardhat/types'; - -export async function getMergedABI( - hre: HardhatRuntimeEnvironment, - primaryABI: string, - secondaryABIs: string[] -): Promise<{abi: any; bytecode: any}> { - const primaryArtifact = await hre.artifacts.readArtifact(primaryABI); - // read all secondary artifacts - const secondariesArtifacts = await Promise.all( - secondaryABIs.map((abi: string) => hre.artifacts.readArtifact(abi)) - ); - const _merged = [...primaryArtifact.abi]; - - // filter events from secondaries artifacts - for (const artifact of secondariesArtifacts) { - _merged.push(...artifact.abi.filter(f => f.type === 'event')); - } - - // remove duplicated events - const merged = _merged.filter((value, index, self) => { - // filter events that meet the following conditions: - // - have the same name - // - have the same number of inputs - // - every input of both events have the same type and name - return ( - index === - self.findIndex(event => { - return ( - // check both events have the same name - event.name === value.name && - // chgeck both events have the same number of inputs - event.inputs.length === value.inputs.length && - // check every input of both events have the same type and name - event.inputs.every((input: any, i: number) => { - return ( - input.type === value.inputs[i].type && - input.name === value.inputs[i].name - ); - }) - ); - }) - ); - }); - - return { - abi: merged, - bytecode: primaryArtifact.bytecode, - }; -} From d292d7107ba5d50181633e6d123d3089e38b1e85 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Tue, 18 Jul 2023 15:34:20 +0200 Subject: [PATCH 03/31] fix: helpers.ts --- packages/contracts/deploy/helpers.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/contracts/deploy/helpers.ts b/packages/contracts/deploy/helpers.ts index 91dea0040..b23bfb20e 100644 --- a/packages/contracts/deploy/helpers.ts +++ b/packages/contracts/deploy/helpers.ts @@ -4,16 +4,16 @@ import {Contract} from 'ethers'; import {HardhatRuntimeEnvironment} from 'hardhat/types'; import IPFS from 'ipfs-http-client'; -import {findEvent} from '../utils/event'; +import {findEvent, findEventTopicLog} from '../utils/event'; import {Operation} from '../utils/types'; import {VersionTag} from '../test/test-utils/psp/types'; import { ENSRegistry__factory, PluginRepoFactory__factory, + PluginRepoRegistry__factory, PluginRepo__factory, } from '../typechain'; import {VersionCreatedEvent} from '../typechain/PluginRepo'; -import {PluginRepoRegisteredEvent} from '../typechain/PluginRepoRegistry'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; // TODO: Add support for L2 such as Arbitrum. (https://discuss.ens.domains/t/register-using-layer-2/688) @@ -223,8 +223,9 @@ export async function createPluginRepo( ); await tx.wait(); - const event = await findEvent( + const event = await findEventTopicLog( tx, + PluginRepoRegistry__factory.createInterface(), 'PluginRepoRegistered' ); const repoAddress = event.args.pluginRepo; From 5acbc827e339a94aa3fb7434014296c39f5f4490 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Tue, 18 Jul 2023 15:40:51 +0200 Subject: [PATCH 04/31] fix: dao-factory.ts --- .../test/framework/dao/dao-factory.ts | 87 ++++++++++--------- 1 file changed, 48 insertions(+), 39 deletions(-) diff --git a/packages/contracts/test/framework/dao/dao-factory.ts b/packages/contracts/test/framework/dao/dao-factory.ts index e7367f688..1ec69c648 100644 --- a/packages/contracts/test/framework/dao/dao-factory.ts +++ b/packages/contracts/test/framework/dao/dao-factory.ts @@ -26,6 +26,7 @@ import { PluginRepo__factory, IProtocolVersion__factory, IERC165__factory, + PluginRepoRegistry__factory, } from '../../../typechain'; import {deployENSSubdomainRegistrar} from '../../test-utils/ens'; @@ -36,7 +37,7 @@ import { } from '../../test-utils/repo'; import adminMetadata from '../../../src/plugins/governance/admin/build-metadata.json'; -import {findEvent} from '../../../utils/event'; +import {findEventTopicLog} from '../../../utils/event'; import {daoExampleURI, deployNewDAO} from '../../test-utils/dao'; import {deployWithProxy} from '../../test-utils/proxy'; import {getAppliedSetupId} from '../../test-utils/psp/hash-helpers'; @@ -52,8 +53,6 @@ import { prepareUninstallation, prepareUpdate, } from '../../test-utils/psp/wrappers'; -import {PluginRepoRegisteredEvent} from '../../../typechain/PluginRepoRegistry'; -import {InstallationPreparedEvent} from '../../../typechain/PluginSetupProcessor'; import {getInterfaceID} from '../../test-utils/interfaces'; import {CURRENT_PROTOCOL_VERSION} from '../../test-utils/protocol-version'; @@ -111,26 +110,25 @@ async function extractInfoFromCreateDaoTx(tx: any): Promise<{ helpers: any; permissions: any; }> { - const data = await tx.wait(); - const {events} = data; - const {dao, creator, subdomain} = events.find( - ({event}: {event: any}) => event === EVENTS.DAORegistered - ).args; - - const { - plugin, - preparedSetupData: {permissions, helpers}, - } = events.find( - ({event}: {event: any}) => event === EVENTS.InstallationPrepared - ).args; + const daoRegisteredEvent = await findEventTopicLog( + tx, + DAORegistry__factory.createInterface(), + EVENTS.DAORegistered + ); + + const installationPreparedEvent = await findEventTopicLog( + tx, + PluginSetupProcessor__factory.createInterface(), + EVENTS.InstallationPrepared + ); return { - dao: dao, - creator: creator, - subdomain: subdomain, - plugin: plugin, - helpers: helpers, - permissions: permissions, + dao: daoRegisteredEvent.args.dao, + creator: daoRegisteredEvent.args.creator, + subdomain: daoRegisteredEvent.args.subdomain, + plugin: installationPreparedEvent.args.plugin, + helpers: installationPreparedEvent.args.preparedSetupData.helpers, + permissions: installationPreparedEvent.args.preparedSetupData.permissions, }; } @@ -249,8 +247,9 @@ describe('DAOFactory: ', function () { '0x00', '0x00' ); - const event = await findEvent( + const event = await findEventTopicLog( tx, + PluginRepoRegistry__factory.createInterface(), EVENTS.PluginRepoRegistered ); pluginSetupMockRepoAddress = event.args.pluginRepo; @@ -535,16 +534,21 @@ describe('DAOFactory: ', function () { const plugins = [plugin1, plugin2]; const tx = await daoFactory.createDao(daoSettings, plugins); - const {events} = await tx.wait(); - let installEventCount = 0; + // Count how often the event was emitted by inspecting the logs + const receipt = await tx.wait(); + const topic = PluginSetupProcessor__factory.createInterface().getEventTopic( + EVENTS.InstallationApplied + ); + + let installationAppliedEventCount = 0; // @ts-ignore - events.forEach(event => { - if (event.event == EVENTS.InstallationApplied) installEventCount++; + receipt.logs.forEach(log => { + if (log.topics[0] == topic) installationAppliedEventCount++; }); - expect(installEventCount).to.equal(2); + expect(installationAppliedEventCount).to.equal(2); }); describe('E2E: Install,Update,Uninstall Plugin through Admin Plugin', async () => { @@ -580,11 +584,12 @@ describe('DAOFactory: ', function () { '0x11', '0x11' ); - let event = await findEvent( + const pluginRepoRegisteredEvent = await findEventTopicLog( tx, + PluginRepoRegistry__factory.createInterface(), EVENTS.PluginRepoRegistered ); - adminPluginRepoAddress = event.args.pluginRepo; + adminPluginRepoAddress = pluginRepoRegisteredEvent.args.pluginRepo; // create dao with admin plugin. const adminPluginRepoPointer: PluginRepoPointer = [ @@ -606,15 +611,19 @@ describe('DAOFactory: ', function () { ); tx = await daoFactory.createDao(daoSettings, [adminPluginInstallation]); { - const event = await findEvent( + const installationPreparedEvent = await findEventTopicLog( tx, + PluginSetupProcessor__factory.createInterface(), EVENTS.InstallationPrepared ); + const adminFactory = new Admin__factory(signers[0]); - adminPlugin = adminFactory.attach(event.args.plugin); + adminPlugin = adminFactory.attach( + installationPreparedEvent.args.plugin + ); const daoFactory = new DAO__factory(signers[0]); - dao = daoFactory.attach(event.args.dao); + dao = daoFactory.attach(installationPreparedEvent.args.dao); } }); @@ -630,15 +639,15 @@ describe('DAOFactory: ', function () { EMPTY_DATA ); - let DAO_INTERFACE = DAO__factory.createInterface(); - let PSP_INTERFACE = PluginSetupProcessor__factory.createInterface(); + const daoInterface = DAO__factory.createInterface(); + const pspInterface = PluginSetupProcessor__factory.createInterface(); // Prepare actions for apply Installation. let applyInstallationActions = [ { to: dao.address, value: 0, - data: DAO_INTERFACE.encodeFunctionData('grant', [ + data: DAO__factory.createInterface().encodeFunctionData('grant', [ dao.address, psp.address, ethers.utils.id('ROOT_PERMISSION'), @@ -647,7 +656,7 @@ describe('DAOFactory: ', function () { { to: psp.address, value: 0, - data: PSP_INTERFACE.encodeFunctionData('applyInstallation', [ + data: pspInterface.encodeFunctionData('applyInstallation', [ dao.address, createApplyInstallationParams( plugin, @@ -686,7 +695,7 @@ describe('DAOFactory: ', function () { { to: dao.address, value: 0, - data: DAO_INTERFACE.encodeFunctionData('grant', [ + data: daoInterface.encodeFunctionData('grant', [ plugin, psp.address, ethers.utils.id('UPGRADE_PLUGIN_PERMISSION'), @@ -695,7 +704,7 @@ describe('DAOFactory: ', function () { { to: psp.address, value: 0, - data: PSP_INTERFACE.encodeFunctionData('applyUpdate', [ + data: pspInterface.encodeFunctionData('applyUpdate', [ dao.address, createApplyUpdateParams( plugin, @@ -727,7 +736,7 @@ describe('DAOFactory: ', function () { { to: psp.address, value: 0, - data: PSP_INTERFACE.encodeFunctionData('applyUninstallation', [ + data: pspInterface.encodeFunctionData('applyUninstallation', [ dao.address, createApplyUninstallationParams( plugin, From 106d0f4b3cf62bc74e4d016a6de02af19f7b5a4d Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Tue, 18 Jul 2023 15:58:12 +0200 Subject: [PATCH 05/31] fix: plugin-setup-processor.ts --- .../plugin/plugin-setup-processor.ts | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/contracts/test/framework/plugin/plugin-setup-processor.ts b/packages/contracts/test/framework/plugin/plugin-setup-processor.ts index d4441929c..5a4f9e220 100644 --- a/packages/contracts/test/framework/plugin/plugin-setup-processor.ts +++ b/packages/contracts/test/framework/plugin/plugin-setup-processor.ts @@ -5,9 +5,6 @@ import {anyValue} from '@nomicfoundation/hardhat-chai-matchers/withArgs'; import { PluginSetupProcessor, - PluginUUPSUpgradeableV1Mock__factory, - PluginUUPSUpgradeableV2Mock__factory, - PluginUUPSUpgradeableV3Mock__factory, PluginUUPSUpgradeableSetupV1Mock, PluginUUPSUpgradeableSetupV1MockBad, PluginUUPSUpgradeableSetupV2Mock, @@ -20,21 +17,25 @@ import { PluginRepoRegistry, PluginRepo, DAO, - PluginUUPSUpgradeableSetupV2Mock__factory, + PluginRepo__factory, + PluginRepoRegistry__factory, + PluginUUPSUpgradeable__factory, + PluginUUPSUpgradeableV1Mock__factory, + PluginUUPSUpgradeableV2Mock__factory, + PluginUUPSUpgradeableV3Mock__factory, + PluginUUPSUpgradeableSetupV1Mock__factory, PluginUUPSUpgradeableSetupV1MockBad__factory, + PluginUUPSUpgradeableSetupV2Mock__factory, PluginUUPSUpgradeableSetupV4Mock__factory, + PluginCloneableSetupV1Mock__factory, PluginCloneableSetupV2Mock__factory, PluginCloneableSetupV1MockBad__factory, - PluginUUPSUpgradeableSetupV1Mock__factory, - PluginCloneableSetupV1Mock__factory, - PluginRepo__factory, - PluginUUPSUpgradeable__factory, } from '../../../typechain'; import {deployENSSubdomainRegistrar} from '../../test-utils/ens'; import {deployNewDAO, ZERO_BYTES32} from '../../test-utils/dao'; import {deployPluginSetupProcessor} from '../../test-utils/plugin-setup-processor'; -import {findEvent} from '../../../utils/event'; +import {findEventTopicLog} from '../../../utils/event'; import {Operation} from '../../../utils/types'; import { @@ -82,7 +83,6 @@ import { uninstallPlugin, } from '../../test-utils/psp/atomic-helpers'; import {UPGRADE_PERMISSIONS} from '../../test-utils/permissions'; -import {PluginRepoRegisteredEvent} from '../../../typechain/PluginRepoRegistry'; const EVENTS = { InstallationPrepared: 'InstallationPrepared', @@ -245,12 +245,13 @@ describe('Plugin Setup Processor', function () { buildMetadata ); - let event = await findEvent( + const PluginRepoRegisteredEvent1 = await findEventTopicLog( tx, + PluginRepoRegistry__factory.createInterface(), EVENTS.PluginRepoRegistered ); const PluginRepo = new PluginRepo__factory(signers[0]); - repoU = PluginRepo.attach(event.args.pluginRepo); + repoU = PluginRepo.attach(PluginRepoRegisteredEvent1.args.pluginRepo); // Add setups await repoU.createVersion(1, setupUV2.address, EMPTY_DATA, EMPTY_DATA); // build 2 @@ -267,11 +268,12 @@ describe('Plugin Setup Processor', function () { buildMetadata ); - event = await findEvent( + const PluginRepoRegisteredEvent2 = await findEventTopicLog( tx, + PluginRepoRegistry__factory.createInterface(), EVENTS.PluginRepoRegistered ); - repoC = PluginRepo.attach(event.args.pluginRepo); + repoC = PluginRepo.attach(PluginRepoRegisteredEvent2.args.pluginRepo); await repoC.createVersion(1, setupCV1Bad.address, EMPTY_DATA, EMPTY_DATA); await repoC.createVersion(1, setupCV2.address, EMPTY_DATA, EMPTY_DATA); }); From ef4d842c84f560c11f925b27a6f76fe4e3edb78b Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Tue, 18 Jul 2023 16:02:46 +0200 Subject: [PATCH 06/31] fix: admin.ts --- .../test/plugins/governance/admin/admin.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/contracts/test/plugins/governance/admin/admin.ts b/packages/contracts/test/plugins/governance/admin/admin.ts index 6c56ad73d..f531cc673 100644 --- a/packages/contracts/test/plugins/governance/admin/admin.ts +++ b/packages/contracts/test/plugins/governance/admin/admin.ts @@ -7,6 +7,7 @@ import { DAO_EVENTS, PROPOSAL_EVENTS, MEMBERSHIP_EVENTS, + findEventTopicLog, } from '../../../../utils/event'; import {deployNewDAO} from '../../../test-utils/dao'; import {getInterfaceID} from '../../../test-utils/interfaces'; @@ -20,9 +21,9 @@ import { IMembership__factory, IPlugin__factory, IProposal__factory, + DAO__factory, } from '../../../../typechain'; import {ProposalCreatedEvent} from '../../../../typechain/Admin'; -import {ExecutedEvent} from '../../../../typechain/DAO'; // Permissions const EXECUTE_PROPOSAL_PERMISSION_ID = ethers.utils.id( @@ -236,7 +237,11 @@ describe('Admin', function () { dummyActions, allowFailureMap ); - const event = await findEvent(tx, DAO_EVENTS.EXECUTED); + const event = await findEventTopicLog( + tx, + DAO__factory.createInterface(), + DAO_EVENTS.EXECUTED + ); expect(event.args.actor).to.equal(plugin.address); expect(event.args.callId).to.equal(toBytes32(proposalId)); @@ -252,8 +257,12 @@ describe('Admin', function () { const proposalId = 1; const tx = await plugin.executeProposal(dummyMetadata, dummyActions, 0); - const event = await findEvent(tx, DAO_EVENTS.EXECUTED); + const event = await findEventTopicLog( + tx, + DAO__factory.createInterface(), + DAO_EVENTS.EXECUTED + ); expect(event.args.callId).to.equal(toBytes32(proposalId)); } }); From 7e909a6b2a5ad69d6fa539f7e1706baf23a0edf7 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Tue, 18 Jul 2023 16:09:26 +0200 Subject: [PATCH 07/31] fix: token-voting.ts --- .../governance/majority-voting/token/token-voting.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts index 55e75c168..c048bb480 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts @@ -5,6 +5,7 @@ import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import { DAO, + DAO__factory, GovernanceERC20Mock, GovernanceERC20Mock__factory, IERC165Upgradeable__factory, @@ -17,6 +18,7 @@ import { } from '../../../../../typechain'; import { findEvent, + findEventTopicLog, DAO_EVENTS, VOTING_EVENTS, PROPOSAL_EVENTS, @@ -47,7 +49,6 @@ import { ProposalCreatedEvent, ProposalExecutedEvent, } from '../../../../../typechain/TokenVoting'; -import {ExecutedEvent} from '../../../../../typechain/DAO'; export const tokenVotingInterface = new ethers.utils.Interface([ 'function initialize(address,tuple(uint8,uint32,uint32,uint64,uint256),address)', @@ -1381,7 +1382,11 @@ describe('TokenVoting', function () { .connect(signers[6]) .vote(id, VoteOption.Yes, true); { - const event = await findEvent(tx, DAO_EVENTS.EXECUTED); + const event = await findEventTopicLog( + tx, + DAO__factory.createInterface(), + DAO_EVENTS.EXECUTED + ); expect(event.args.actor).to.equal(voting.address); expect(event.args.callId).to.equal(toBytes32(id)); From b99252481d763d7773e0c35f5a29523f74323d2c Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Tue, 18 Jul 2023 16:09:36 +0200 Subject: [PATCH 08/31] fix: addresslist-voting.ts --- .../majority-voting/addresslist/addresslist-voting.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts index 155f722ff..6ff1c4f83 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts @@ -7,6 +7,7 @@ import { AddresslistVoting__factory, Addresslist__factory, DAO, + DAO__factory, IERC165Upgradeable__factory, IMajorityVoting__factory, IMembership__factory, @@ -15,6 +16,7 @@ import { } from '../../../../../typechain'; import { findEvent, + findEventTopicLog, DAO_EVENTS, VOTING_EVENTS, PROPOSAL_EVENTS, @@ -40,7 +42,6 @@ import {UPGRADE_PERMISSIONS} from '../../../../test-utils/permissions'; import {deployWithProxy} from '../../../../test-utils/proxy'; import {getInterfaceID} from '../../../../test-utils/interfaces'; import {majorityVotingBaseInterface} from '../majority-voting'; -import {ExecutedEvent} from '../../../../../typechain/DAO'; import { ProposalCreatedEvent, ProposalExecutedEvent, @@ -1025,7 +1026,11 @@ describe('AddresslistVoting', function () { .connect(signers[6]) .vote(id, VoteOption.Abstain, true); { - const event = await findEvent(tx, DAO_EVENTS.EXECUTED); + const event = await findEventTopicLog( + tx, + DAO__factory.createInterface(), + DAO_EVENTS.EXECUTED + ); expect(event.args.actor).to.equal(voting.address); expect(event.args.callId).to.equal(toBytes32(id)); From bcce07f87dea809aed953852fa370b3c2fdbb101 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Tue, 18 Jul 2023 16:11:20 +0200 Subject: [PATCH 09/31] feature: improved error --- packages/contracts/utils/event.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts/utils/event.ts b/packages/contracts/utils/event.ts index b6d54f743..48f09635c 100644 --- a/packages/contracts/utils/event.ts +++ b/packages/contracts/utils/event.ts @@ -21,7 +21,7 @@ export async function findEventTopicLog( const topic = iface.getEventTopic(eventName); const log = receipt.logs.find(x => x.topics[0] === topic); if (!log) { - throw new Error(`No logs found for this event ${eventName} topic.`); + throw new Error(`No logs found for the topic of event "${eventName}".`); } return iface.parseLog(log); } From 5a6ea503f631a680fe487b63b889d2c68b0820e4 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Tue, 18 Jul 2023 16:17:02 +0200 Subject: [PATCH 10/31] fix: multisig.ts --- .../plugins/governance/multisig/multisig.ts | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/contracts/test/plugins/governance/multisig/multisig.ts b/packages/contracts/test/plugins/governance/multisig/multisig.ts index 030621978..127829571 100644 --- a/packages/contracts/test/plugins/governance/multisig/multisig.ts +++ b/packages/contracts/test/plugins/governance/multisig/multisig.ts @@ -6,6 +6,7 @@ import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import { Addresslist__factory, DAO, + DAO__factory, IERC165Upgradeable__factory, IMembership__factory, IMultisig__factory, @@ -16,6 +17,7 @@ import { } from '../../../../typechain'; import { findEvent, + findEventTopicLog, DAO_EVENTS, PROPOSAL_EVENTS, MULTISIG_EVENTS, @@ -1168,21 +1170,33 @@ describe('Multisig', function () { // `tryExecution` is turned on but the vote is not decided yet let tx = await multisig.connect(signers[1]).approve(id, true); await expect( - findEvent(tx, DAO_EVENTS.EXECUTED) - ).to.rejectedWith('Event Executed not found in TX.'); + findEventTopicLog( + tx, + DAO__factory.createInterface(), + DAO_EVENTS.EXECUTED + ) + ).to.rejectedWith('No logs found for the topic of event "Executed".'); expect(await multisig.canExecute(id)).to.equal(false); // `tryExecution` is turned off and the vote is decided tx = await multisig.connect(signers[2]).approve(id, false); await expect( - findEvent(tx, DAO_EVENTS.EXECUTED) - ).to.rejectedWith('Event Executed not found in TX.'); + findEventTopicLog( + tx, + DAO__factory.createInterface(), + DAO_EVENTS.EXECUTED + ) + ).to.rejectedWith('No logs found for the topic of event "Executed".'); // `tryEarlyExecution` is turned on and the vote is decided tx = await multisig.connect(signers[3]).approve(id, true); { - const event = await findEvent(tx, DAO_EVENTS.EXECUTED); + const event = await findEventTopicLog( + tx, + DAO__factory.createInterface(), + DAO_EVENTS.EXECUTED + ); expect(event.args.actor).to.equal(multisig.address); expect(event.args.callId).to.equal(toBytes32(id)); From 323c8d97124ffaab952d9c0af850d20efa0383a4 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Wed, 19 Jul 2023 17:50:32 +0200 Subject: [PATCH 11/31] feature: refactored upgradeability tests --- packages/contracts/src/test/Migration.sol | 8 + packages/contracts/test/core/dao/dao.ts | 49 +- .../test/framework/dao/dao-registry.ts | 43 +- .../framework/plugin/plugin-repo-registry.ts | 45 +- .../test/framework/plugin/plugin-repo.ts | 959 +++++++++--------- .../utils/ens/ens-subdomain-registry.ts | 70 +- .../addresslist/addresslist-voting.ts | 65 +- .../majority-voting/token/token-voting.ts | 64 +- .../plugins/governance/multisig/multisig.ts | 65 +- .../token/distribution/merkle-distributor.ts | 17 - .../token/distribution/merkle-minter.ts | 12 - .../test/test-utils/uups-upgradeable.ts | 161 +-- packages/contracts/utils/storage.ts | 4 +- 13 files changed, 839 insertions(+), 723 deletions(-) diff --git a/packages/contracts/src/test/Migration.sol b/packages/contracts/src/test/Migration.sol index 3304763a3..f70702516 100644 --- a/packages/contracts/src/test/Migration.sol +++ b/packages/contracts/src/test/Migration.sol @@ -20,3 +20,11 @@ pragma solidity 0.8.17; */ import {DAO as DAO_v1_0_0} from "@aragon/osx-v1.0.1/core/dao/DAO.sol"; +import {DAORegistry as DAORegistry_v1_0_0} from "@aragon/osx-v1.0.1/framework/dao/DAORegistry.sol"; +import {PluginRepo as PluginRepo_v1_0_0} from "@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepo.sol"; +import {PluginRepoRegistry as PluginRepoRegistry_v1_0_0} from "@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepoRegistry.sol"; +import {ENSSubdomainRegistrar as ENSSubdomainRegistrar_v1_0_0} from "@aragon/osx-v1.0.1/framework/utils/ens/ENSSubdomainRegistrar.sol"; + +import {TokenVoting as TokenVoting_v1_0_0} from "@aragon/osx-v1.0.1/plugins/governance/majority-voting/token/TokenVoting.sol"; +import {AddresslistVoting as AddresslistVoting_v1_0_0} from "@aragon/osx-v1.0.1/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol"; +import {Multisig as Multisig_v1_0_0} from "@aragon/osx-v1.0.1/plugins/governance/multisig/Multisig.sol"; diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts index fb2a19d06..b8bcbcd2f 100644 --- a/packages/contracts/test/core/dao/dao.ts +++ b/packages/contracts/test/core/dao/dao.ts @@ -1,5 +1,6 @@ import chai, {expect} from 'chai'; import {ethers} from 'hardhat'; +import {ContractFactory} from 'ethers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import { @@ -21,6 +22,9 @@ import { IEIP4824__factory, IProtocolVersion__factory, } from '../../../typechain'; +import {DAO__factory as DAO_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/core/dao/DAO.sol'; + +import {upgradeManagingContract} from '../../test-utils/uups-upgradeable'; import {findEvent, DAO_EVENTS} from '../../../utils/event'; import {flipBit} from '../../test-utils/bitmap'; @@ -37,7 +41,6 @@ import {OZ_ERRORS} from '../../test-utils/error'; import {smock} from '@defi-wonderland/smock'; import {deployWithProxy} from '../../test-utils/proxy'; import {UNREGISTERED_INTERFACE_RETURN} from './callback-handler'; -import {shouldUpgradeCorrectly} from '../../test-utils/uups-upgradeable'; import {UPGRADE_PERMISSIONS} from '../../test-utils/permissions'; import {ZERO_BYTES32, daoExampleURI} from '../../test-utils/dao'; import {ExecutedEvent} from '../../../typechain/DAO'; @@ -94,10 +97,12 @@ describe('DAO', function () { let dao: DAO; let DAO: DAO__factory; - beforeEach(async function () { + before(async () => { signers = await ethers.getSigners(); ownerAddress = await signers[0].getAddress(); + }); + beforeEach(async function () { DAO = new DAO__factory(signers[0]); dao = await deployWithProxy(DAO); await dao.initialize( @@ -140,19 +145,8 @@ describe('DAO', function () { PERMISSION_IDS.REGISTER_STANDARD_CALLBACK_PERMISSION_ID ), ]); - - this.upgrade = { - contract: dao, - dao: dao, - user: signers[8], - }; }); - shouldUpgradeCorrectly( - UPGRADE_PERMISSIONS.UPGRADE_DAO_PERMISSION_ID, - 'Unauthorized' - ); - it('does not support the empty interface', async () => { expect(await dao.supportsInterface('0xffffffff')).to.be.false; }); @@ -327,6 +321,34 @@ describe('DAO', function () { }); }); + describe('Upgrades', async () => { + let legacyContractFactory: ContractFactory; + let currentContractFactory: ContractFactory; + + before(() => { + currentContractFactory = new DAO__factory(signers[0]); + }); + + it('from v1.0.0', async () => { + legacyContractFactory = new DAO_V1_0_0__factory(signers[0]); + + await upgradeManagingContract( + signers[0], + signers[1], + { + metadata: dummyMetadata1, + initialOwner: signers[0].address, + trustedForwarder: dummyAddress1, + daoURI: daoExampleURI, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_DAO_PERMISSION_ID + ); + }); + }); + describe('ERC-165', async () => { it('does not support the empty interface', async () => { expect(await dao.supportsInterface('0xffffffff')).to.be.false; @@ -1147,7 +1169,6 @@ describe('DAO', function () { }); it('should allow only `SET_SIGNATURE_VALIDATOR_PERMISSION_ID` to set validator', async () => { - const signers = await ethers.getSigners(); await expect( dao .connect(signers[2]) diff --git a/packages/contracts/test/framework/dao/dao-registry.ts b/packages/contracts/test/framework/dao/dao-registry.ts index 272c75ab1..5aa12fbca 100644 --- a/packages/contracts/test/framework/dao/dao-registry.ts +++ b/packages/contracts/test/framework/dao/dao-registry.ts @@ -1,5 +1,6 @@ import {expect} from 'chai'; import {ethers} from 'hardhat'; +import {ContractFactory} from 'ethers'; import {ensDomainHash, ensLabelHash} from '../../../utils/ens'; import { @@ -8,12 +9,14 @@ import { DAORegistry__factory, ENSSubdomainRegistrar, } from '../../../typechain'; +import {DAORegistry__factory as DAORegistry_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/framework/dao/DAORegistry.sol'; + import {deployNewDAO} from '../../test-utils/dao'; import {deployENSSubdomainRegistrar} from '../../test-utils/ens'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {deployWithProxy} from '../../test-utils/proxy'; -import {shouldUpgradeCorrectly} from '../../test-utils/uups-upgradeable'; import {UPGRADE_PERMISSIONS} from '../../test-utils/permissions'; +import {upgradeManagedContract} from '../../test-utils/uups-upgradeable'; const EVENTS = { DAORegistered: 'DAORegistered', @@ -79,19 +82,8 @@ describe('DAORegistry', function () { daoRegistry.address, REGISTER_ENS_SUBDOMAIN_PERMISSION_ID ); - - this.upgrade = { - contract: daoRegistry, - dao: managingDao, - user: signers[8], - }; }); - shouldUpgradeCorrectly( - UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID, - 'DaoUnauthorized' - ); - it('succeeds even if the dao subdomain is empty', async function () { await expect(daoRegistry.register(targetDao.address, ownerAddress, '')).to .not.be.reverted; @@ -252,4 +244,31 @@ describe('DAORegistry', function () { } }).timeout(120000); }); + + describe('Upgrades', () => { + let legacyContractFactory: ContractFactory; + let currentContractFactory: ContractFactory; + + before(() => { + currentContractFactory = new DAORegistry__factory(signers[0]); + }); + + it('from v1.0.0', async () => { + legacyContractFactory = new DAORegistry_V1_0_0__factory(signers[0]); + + await upgradeManagedContract( + signers[0], + signers[1], + managingDao, + { + dao: managingDao.address, + ensSubdomainRegistrar: ensSubdomainRegistrar.address, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID + ); + }); + }); }); diff --git a/packages/contracts/test/framework/plugin/plugin-repo-registry.ts b/packages/contracts/test/framework/plugin/plugin-repo-registry.ts index 40bdb1042..2d6cc34e1 100644 --- a/packages/contracts/test/framework/plugin/plugin-repo-registry.ts +++ b/packages/contracts/test/framework/plugin/plugin-repo-registry.ts @@ -1,5 +1,6 @@ import {expect} from 'chai'; import {ethers} from 'hardhat'; +import {ContractFactory} from 'ethers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import { @@ -9,13 +10,15 @@ import { PluginRepoRegistry, PluginRepoRegistry__factory, } from '../../../typechain'; +import {PluginRepoRegistry__factory as PluginRepoRegistry_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepoRegistry.sol'; + import {deployNewDAO} from '../../test-utils/dao'; import {deployNewPluginRepo} from '../../test-utils/repo'; import {deployENSSubdomainRegistrar} from '../../test-utils/ens'; import {ensDomainHash} from '../../../utils/ens'; import {deployWithProxy} from '../../test-utils/proxy'; -import {shouldUpgradeCorrectly} from '../../test-utils/uups-upgradeable'; import {UPGRADE_PERMISSIONS} from '../../test-utils/permissions'; +import {upgradeManagedContract} from '../../test-utils/uups-upgradeable'; const EVENTS = { PluginRepoRegistered: 'PluginRepoRegistered', @@ -83,19 +86,8 @@ describe('PluginRepoRegistry', function () { pluginRepoRegistry.address, REGISTER_ENS_SUBDOMAIN_PERMISSION_ID ); - - this.upgrade = { - contract: pluginRepoRegistry, - dao: managingDAO, - user: signers[8], - }; }); - shouldUpgradeCorrectly( - UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID, - 'DaoUnauthorized' - ); - it('successfully sets subdomainregistrar', async () => { expect(await pluginRepoRegistry.subdomainRegistrar()).to.equal( ensSubdomainRegistrar.address @@ -261,4 +253,33 @@ describe('PluginRepoRegistry', function () { } }).timeout(120000); }); + + describe('Upgrades', () => { + let legacyContractFactory: ContractFactory; + let currentContractFactory: ContractFactory; + + before(() => { + currentContractFactory = new PluginRepoRegistry__factory(signers[0]); + }); + + it('from v1.0.0', async () => { + legacyContractFactory = new PluginRepoRegistry_V1_0_0__factory( + signers[0] + ); + + await upgradeManagedContract( + signers[0], + signers[1], + managingDAO, + { + dao: managingDAO.address, + ensSubdomainRegistrar: ensSubdomainRegistrar.address, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID + ); + }); + }); }); diff --git a/packages/contracts/test/framework/plugin/plugin-repo.ts b/packages/contracts/test/framework/plugin/plugin-repo.ts index ba629afbf..ee43d9dbf 100644 --- a/packages/contracts/test/framework/plugin/plugin-repo.ts +++ b/packages/contracts/test/framework/plugin/plugin-repo.ts @@ -3,25 +3,28 @@ import {expect} from 'chai'; import {ethers} from 'hardhat'; +import {ContractFactory} from 'ethers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {keccak256, solidityPack} from 'ethers/lib/utils'; import { PluginRepo, + PluginRepo__factory, PluginUUPSUpgradeableSetupV1Mock, PlaceholderSetup__factory, TestPlugin__factory, IERC165__factory, IPluginRepo__factory, IProtocolVersion__factory, - UUPSUpgradeable__factory, } from '../../../typechain'; +import {PluginRepo__factory as PluginRepo_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepo.sol'; + +import {upgradeManagingContract} from '../../test-utils/uups-upgradeable'; import { deployMockPluginSetup, deployNewPluginRepo, } from '../../test-utils/repo'; -import {shouldUpgradeCorrectly} from '../../test-utils/uups-upgradeable'; import {UPGRADE_PERMISSIONS} from '../../test-utils/permissions'; import {ZERO_BYTES32} from '../../test-utils/dao'; import {getInterfaceID} from '../../test-utils/interfaces'; @@ -49,547 +52,579 @@ describe('PluginRepo', function () { // deploy pluging factory mock pluginSetupMock = await deployMockPluginSetup(signers[0]); - - this.upgrade = { - contract: pluginRepo, - dao: pluginRepo, - user: signers[8], - }; - }); - - shouldUpgradeCorrectly( - UPGRADE_PERMISSIONS.UPGRADE_REPO_PERMISSION_ID, - 'Unauthorized' - ); - - it('initializes correctly', async () => { - const permissions = [ - ethers.utils.id('MAINTAINER_PERMISSION'), - ethers.utils.id('UPGRADE_REPO_PERMISSION'), - ethers.utils.id('ROOT_PERMISSION'), - ]; - - for (let i = 0; i < permissions.length; i++) { - expect( - await pluginRepo.isGranted( - pluginRepo.address, - ownerAddress, - permissions[i], - '0x' - ) - ).to.be.true; - } }); - describe('ERC-165', async () => { - it('does not support the empty interface', async () => { - expect(await pluginRepo.supportsInterface('0xffffffff')).to.be.false; + describe('Initialize', () => { + it('initializes correctly', async () => { + const permissions = [ + ethers.utils.id('MAINTAINER_PERMISSION'), + ethers.utils.id('UPGRADE_REPO_PERMISSION'), + ethers.utils.id('ROOT_PERMISSION'), + ]; + + for (let i = 0; i < permissions.length; i++) { + expect( + await pluginRepo.isGranted( + pluginRepo.address, + ownerAddress, + permissions[i], + '0x' + ) + ).to.be.true; + } }); - it('supports the `IERC165` interface', async () => { - const iface = IERC165__factory.createInterface(); - expect(await pluginRepo.supportsInterface(getInterfaceID(iface))).to.be - .true; - }); + describe('Upgrades', () => { + let legacyContractFactory: ContractFactory; + let currentContractFactory: ContractFactory; - it('supports the `IPluginRepo` interface', async () => { - const iface = IPluginRepo__factory.createInterface(); - expect(getInterfaceID(iface)).to.equal('0xd4321b40'); // the interfaceID from IPluginRepo v1.0.0 - expect(await pluginRepo.supportsInterface(getInterfaceID(iface))).to.be - .true; - }); + before(() => { + currentContractFactory = new PluginRepo__factory(signers[0]); + }); - it('supports the `IProtocolVersion` interface', async () => { - const iface = IProtocolVersion__factory.createInterface(); - expect(await pluginRepo.supportsInterface(getInterfaceID(iface))).to.be - .true; + it('from v1.0.0', async () => { + legacyContractFactory = new PluginRepo_V1_0_0__factory(signers[0]); + + await upgradeManagingContract( + signers[0], + signers[1], + { + initialOwner: signers[0].address, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_REPO_PERMISSION_ID + ); + }); }); - }); - describe('Protocol version', async () => { - it('returns the current protocol version', async () => { - expect(await pluginRepo.protocolVersion()).to.deep.equal( - CURRENT_PROTOCOL_VERSION - ); - }); - }); + describe('ERC-165', async () => { + it('does not support the empty interface', async () => { + expect(await pluginRepo.supportsInterface('0xffffffff')).to.be.false; + }); - describe('CreateVersion: ', async () => { - it('reverts if the caller does not have permission', async () => { - await expect( - pluginRepo - .connect(signers[2]) - .createVersion(1, pluginSetupMock.address, emptyBytes, emptyBytes) - ) - .to.be.revertedWithCustomError(pluginRepo, 'Unauthorized') - .withArgs( - pluginRepo.address, - signers[2].address, - MAINTAINER_PERMISSION_ID - ); - }); + it('supports the `IERC165` interface', async () => { + const iface = IERC165__factory.createInterface(); + expect(await pluginRepo.supportsInterface(getInterfaceID(iface))).to.be + .true; + }); - it('fails if the plugin setup does not support the `IPluginSetup` interface', async function () { - // If EOA Address is passed - await expect( - pluginRepo.createVersion(1, ownerAddress, emptyBytes, emptyBytes) - ).to.be.revertedWithCustomError( - pluginRepo, - 'InvalidPluginSetupInterface' - ); - - // If a contract is passed, but doesn't support `IPluginSetup`. - await expect( - pluginRepo.createVersion(1, pluginRepo.address, emptyBytes, emptyBytes) - ).to.be.revertedWithCustomError( - pluginRepo, - 'InvalidPluginSetupInterface' - ); - - // If a contract is passed, but doesn't have `supportsInterface` signature described in the contract. - const randomContract = await new TestPlugin__factory(signers[0]).deploy(); - await expect( - pluginRepo.createVersion( - 1, - randomContract.address, - emptyBytes, - emptyBytes - ) - ).to.be.revertedWithCustomError( - pluginRepo, - 'InvalidPluginSetupInterface' - ); - }); + it('supports the `IPluginRepo` interface', async () => { + const iface = IPluginRepo__factory.createInterface(); + expect(getInterfaceID(iface)).to.equal('0xd4321b40'); // the interfaceID from IPluginRepo v1.0.0 + expect(await pluginRepo.supportsInterface(getInterfaceID(iface))).to.be + .true; + }); - it('fails if the release number is 0', async () => { - await expect( - pluginRepo.createVersion( - 0, - pluginSetupMock.address, - emptyBytes, - emptyBytes - ) - ).to.be.revertedWithCustomError(pluginRepo, 'ReleaseZeroNotAllowed'); + it('supports the `IProtocolVersion` interface', async () => { + const iface = IProtocolVersion__factory.createInterface(); + expect(await pluginRepo.supportsInterface(getInterfaceID(iface))).to.be + .true; + }); }); - it('fails if the release is incremented by more than 1', async () => { - await pluginRepo.createVersion( - 1, - pluginSetupMock.address, - BUILD_METADATA, - RELEASE_METADATA - ); - - await expect( - pluginRepo.createVersion( - 3, - pluginSetupMock.address, - BUILD_METADATA, - RELEASE_METADATA - ) - ).to.be.revertedWithCustomError(pluginRepo, 'InvalidReleaseIncrement'); + describe('Protocol version', async () => { + it('returns the current protocol version', async () => { + expect(await pluginRepo.protocolVersion()).to.deep.equal( + CURRENT_PROTOCOL_VERSION + ); + }); }); - it('fails for the first release, if `releaseMetadata` is empty', async () => { - await expect( - pluginRepo.createVersion( - 1, - pluginSetupMock.address, - BUILD_METADATA, - '0x' + describe('CreateVersion: ', async () => { + it('reverts if the caller does not have permission', async () => { + await expect( + pluginRepo + .connect(signers[2]) + .createVersion(1, pluginSetupMock.address, emptyBytes, emptyBytes) ) - ).to.be.revertedWithCustomError(pluginRepo, 'EmptyReleaseMetadata'); - }); + .to.be.revertedWithCustomError(pluginRepo, 'Unauthorized') + .withArgs( + pluginRepo.address, + signers[2].address, + MAINTAINER_PERMISSION_ID + ); + }); - it('fails if the same plugin setup exists in another release', async () => { - const pluginSetup_1 = await deployMockPluginSetup(signers[0]); - const pluginSetup_2 = await deployMockPluginSetup(signers[0]); - - // create release 1 - await pluginRepo.createVersion( - 1, - pluginSetup_1.address, - BUILD_METADATA, - RELEASE_METADATA - ); - - // create release 2 - await pluginRepo.createVersion( - 2, - pluginSetup_2.address, - BUILD_METADATA, - RELEASE_METADATA - ); - - // release 3 should fail as it's using the same plugin of first release - await expect( - pluginRepo.createVersion( - 3, - pluginSetup_1.address, - BUILD_METADATA, - RELEASE_METADATA - ) - ) - .to.be.revertedWithCustomError( + it('fails if the plugin setup does not support the `IPluginSetup` interface', async function () { + // If EOA Address is passed + await expect( + pluginRepo.createVersion(1, ownerAddress, emptyBytes, emptyBytes) + ).to.be.revertedWithCustomError( pluginRepo, - 'PluginSetupAlreadyInPreviousRelease' - ) - .withArgs(1, 1, pluginSetup_1.address); + 'InvalidPluginSetupInterface' + ); - // release 3 should fail as it's using the same plugin of second release - await expect( - pluginRepo.createVersion( - 3, - pluginSetup_2.address, - BUILD_METADATA, - RELEASE_METADATA - ) - ) - .to.be.revertedWithCustomError( + // If a contract is passed, but doesn't support `IPluginSetup`. + await expect( + pluginRepo.createVersion( + 1, + pluginRepo.address, + emptyBytes, + emptyBytes + ) + ).to.be.revertedWithCustomError( pluginRepo, - 'PluginSetupAlreadyInPreviousRelease' - ) - .withArgs(2, 1, pluginSetup_2.address); - }); + 'InvalidPluginSetupInterface' + ); - it('successfully creates a version and emits the correct events', async () => { - await expect( - pluginRepo.createVersion( - 1, - pluginSetupMock.address, - BUILD_METADATA, - RELEASE_METADATA - ) - ) - .to.emit(pluginRepo, 'VersionCreated') - .withArgs(1, 1, pluginSetupMock.address, BUILD_METADATA) - .to.emit(pluginRepo, 'ReleaseMetadataUpdated') - .withArgs(1, RELEASE_METADATA); - }); + // If a contract is passed, but doesn't have `supportsInterface` signature described in the contract. + const randomContract = await new TestPlugin__factory( + signers[0] + ).deploy(); + await expect( + pluginRepo.createVersion( + 1, + randomContract.address, + emptyBytes, + emptyBytes + ) + ).to.be.revertedWithCustomError( + pluginRepo, + 'InvalidPluginSetupInterface' + ); + }); + + it('fails if the release number is 0', async () => { + await expect( + pluginRepo.createVersion( + 0, + pluginSetupMock.address, + emptyBytes, + emptyBytes + ) + ).to.be.revertedWithCustomError(pluginRepo, 'ReleaseZeroNotAllowed'); + }); - it('correctly increases and emits the build number', async () => { - await expect( - pluginRepo.createVersion( + it('fails if the release is incremented by more than 1', async () => { + await pluginRepo.createVersion( 1, pluginSetupMock.address, BUILD_METADATA, RELEASE_METADATA - ) - ) - .to.emit(pluginRepo, 'VersionCreated') - .withArgs(1, 1, pluginSetupMock.address, BUILD_METADATA); + ); - expect(await pluginRepo.buildCount(1)).to.equal(1); + await expect( + pluginRepo.createVersion( + 3, + pluginSetupMock.address, + BUILD_METADATA, + RELEASE_METADATA + ) + ).to.be.revertedWithCustomError(pluginRepo, 'InvalidReleaseIncrement'); + }); - await expect( - pluginRepo.createVersion( - 1, - pluginSetupMock.address, - BUILD_METADATA, - RELEASE_METADATA - ) - ) - .to.emit(pluginRepo, 'VersionCreated') - .withArgs(1, 2, pluginSetupMock.address, BUILD_METADATA); + it('fails for the first release, if `releaseMetadata` is empty', async () => { + await expect( + pluginRepo.createVersion( + 1, + pluginSetupMock.address, + BUILD_METADATA, + '0x' + ) + ).to.be.revertedWithCustomError(pluginRepo, 'EmptyReleaseMetadata'); + }); - expect(await pluginRepo.buildCount(1)).to.equal(2); - }); + it('fails if the same plugin setup exists in another release', async () => { + const pluginSetup_1 = await deployMockPluginSetup(signers[0]); + const pluginSetup_2 = await deployMockPluginSetup(signers[0]); - it('correctly increases and emits release number', async () => { - await expect( - pluginRepo.createVersion( + // create release 1 + await pluginRepo.createVersion( 1, - pluginSetupMock.address, + pluginSetup_1.address, BUILD_METADATA, RELEASE_METADATA - ) - ) - .to.emit(pluginRepo, 'VersionCreated') - .withArgs(1, 1, pluginSetupMock.address, BUILD_METADATA); - - expect(await pluginRepo.latestRelease()).to.equal(1); - - // don't repeat the same plugin setup in the 2nd release - // otherwise it will revert. - const pluginSetupMock_2 = await deployMockPluginSetup(signers[0]); + ); - await expect( - pluginRepo.createVersion( + // create release 2 + await pluginRepo.createVersion( 2, - pluginSetupMock_2.address, + pluginSetup_2.address, BUILD_METADATA, RELEASE_METADATA - ) - ) - .to.emit(pluginRepo, 'VersionCreated') - .withArgs(2, 1, pluginSetupMock_2.address, BUILD_METADATA); - - expect(await pluginRepo.latestRelease()).to.equal(2); - }); - - it('succeeds if release already exists and release metadata is empty', async () => { - await pluginRepo.createVersion( - 1, - pluginSetupMock.address, - BUILD_METADATA, - RELEASE_METADATA - ); + ); - await expect( - pluginRepo.createVersion( - 1, - pluginSetupMock.address, - BUILD_METADATA, - '0x' + // release 3 should fail as it's using the same plugin of first release + await expect( + pluginRepo.createVersion( + 3, + pluginSetup_1.address, + BUILD_METADATA, + RELEASE_METADATA + ) ) - ).to.not.emit(pluginRepo, 'ReleaseMetadataUpdated'); - }); - - it('allows to create placeholder builds for the same release', async () => { - const PlaceholderSetup = new PlaceholderSetup__factory(signers[0]); - const placeholder1 = await PlaceholderSetup.deploy(); - const placeholder2 = await PlaceholderSetup.deploy(); + .to.be.revertedWithCustomError( + pluginRepo, + 'PluginSetupAlreadyInPreviousRelease' + ) + .withArgs(1, 1, pluginSetup_1.address); - // Release 1 - await expect( - pluginRepo.createVersion( - 1, - placeholder1.address, - ZERO_BYTES32, - ZERO_BYTES32 + // release 3 should fail as it's using the same plugin of second release + await expect( + pluginRepo.createVersion( + 3, + pluginSetup_2.address, + BUILD_METADATA, + RELEASE_METADATA + ) ) - ) - .to.emit(pluginRepo, 'VersionCreated') - .withArgs(1, 1, placeholder1.address, ZERO_BYTES32); + .to.be.revertedWithCustomError( + pluginRepo, + 'PluginSetupAlreadyInPreviousRelease' + ) + .withArgs(2, 1, pluginSetup_2.address); + }); - await expect( - pluginRepo.createVersion( - 1, - placeholder1.address, - ZERO_BYTES32, - ZERO_BYTES32 + it('successfully creates a version and emits the correct events', async () => { + await expect( + pluginRepo.createVersion( + 1, + pluginSetupMock.address, + BUILD_METADATA, + RELEASE_METADATA + ) ) - ) - .to.emit(pluginRepo, 'VersionCreated') - .withArgs(1, 2, placeholder1.address, ZERO_BYTES32); + .to.emit(pluginRepo, 'VersionCreated') + .withArgs(1, 1, pluginSetupMock.address, BUILD_METADATA) + .to.emit(pluginRepo, 'ReleaseMetadataUpdated') + .withArgs(1, RELEASE_METADATA); + }); - // Release 2 - await expect( - pluginRepo.createVersion( - 2, - placeholder2.address, - ZERO_BYTES32, - ZERO_BYTES32 + it('correctly increases and emits the build number', async () => { + await expect( + pluginRepo.createVersion( + 1, + pluginSetupMock.address, + BUILD_METADATA, + RELEASE_METADATA + ) ) - ) - .to.emit(pluginRepo, 'VersionCreated') - .withArgs(2, 1, placeholder2.address, ZERO_BYTES32); + .to.emit(pluginRepo, 'VersionCreated') + .withArgs(1, 1, pluginSetupMock.address, BUILD_METADATA); - await expect( - pluginRepo.createVersion( - 2, - placeholder2.address, - ZERO_BYTES32, - ZERO_BYTES32 + expect(await pluginRepo.buildCount(1)).to.equal(1); + + await expect( + pluginRepo.createVersion( + 1, + pluginSetupMock.address, + BUILD_METADATA, + RELEASE_METADATA + ) ) - ) - .to.emit(pluginRepo, 'VersionCreated') - .withArgs(2, 2, placeholder2.address, ZERO_BYTES32); - }); - }); + .to.emit(pluginRepo, 'VersionCreated') + .withArgs(1, 2, pluginSetupMock.address, BUILD_METADATA); - describe('updateReleaseMetadata', async () => { - it('reverts if caller does not have permission', async () => { - await expect( - pluginRepo - .connect(signers[2]) - .updateReleaseMetadata(1, RELEASE_METADATA) - ) - .to.be.revertedWithCustomError(pluginRepo, 'Unauthorized') - .withArgs( - pluginRepo.address, - signers[2].address, - MAINTAINER_PERMISSION_ID - ); - }); - it('reverts if release is 0', async () => { - await expect( - pluginRepo.updateReleaseMetadata(0, emptyBytes) - ).to.be.revertedWithCustomError(pluginRepo, 'ReleaseZeroNotAllowed'); - }); + expect(await pluginRepo.buildCount(1)).to.equal(2); + }); - it('reverts if release does not exist', async () => { - await expect( - pluginRepo.updateReleaseMetadata(1, emptyBytes) - ).to.be.revertedWithCustomError(pluginRepo, 'ReleaseDoesNotExist'); - }); + it('correctly increases and emits release number', async () => { + await expect( + pluginRepo.createVersion( + 1, + pluginSetupMock.address, + BUILD_METADATA, + RELEASE_METADATA + ) + ) + .to.emit(pluginRepo, 'VersionCreated') + .withArgs(1, 1, pluginSetupMock.address, BUILD_METADATA); - it('reverts if metadata length is 0', async () => { - await pluginRepo.createVersion( - 1, - pluginSetupMock.address, - BUILD_METADATA, - RELEASE_METADATA - ); - await expect( - pluginRepo.updateReleaseMetadata(1, '0x') - ).to.be.revertedWithCustomError(pluginRepo, 'EmptyReleaseMetadata'); - }); + expect(await pluginRepo.latestRelease()).to.equal(1); - it('successfuly updates metadata for the release that already exists', async () => { - await pluginRepo.createVersion( - 1, - pluginSetupMock.address, - BUILD_METADATA, - RELEASE_METADATA - ); - await expect(pluginRepo.updateReleaseMetadata(1, '0x11')) - .to.emit(pluginRepo, 'ReleaseMetadataUpdated') - .withArgs(1, '0x11'); - }); - }); + // don't repeat the same plugin setup in the 2nd release + // otherwise it will revert. + const pluginSetupMock_2 = await deployMockPluginSetup(signers[0]); - describe('Different types of getVersions:', async () => { - // R - release, B - build - let pluginSetup_R1_B1: PluginUUPSUpgradeableSetupV1Mock; - let pluginSetup_R1_B2: PluginUUPSUpgradeableSetupV1Mock; - let pluginSetup_R2_B1: PluginUUPSUpgradeableSetupV1Mock; - let BUILD_METADATA_R1_B1 = BUILD_METADATA; - let BUILD_METADATA_R1_B2 = `${BUILD_METADATA}11`; - let BUILD_METADATA_R2_B1 = `${BUILD_METADATA}1111`; - - beforeEach(async () => { - pluginSetup_R1_B1 = pluginSetupMock; - pluginSetup_R1_B2 = await deployMockPluginSetup(signers[0]); - pluginSetup_R2_B1 = await deployMockPluginSetup(signers[0]); - - await pluginRepo.createVersion( - 1, - pluginSetup_R1_B1.address, - BUILD_METADATA_R1_B1, - RELEASE_METADATA - ); - - await pluginRepo.createVersion( - 1, - pluginSetup_R1_B2.address, - BUILD_METADATA_R1_B2, - RELEASE_METADATA - ); - - await pluginRepo.createVersion( - 2, - pluginSetup_R2_B1.address, - BUILD_METADATA_R2_B1, - RELEASE_METADATA - ); - }); + await expect( + pluginRepo.createVersion( + 2, + pluginSetupMock_2.address, + BUILD_METADATA, + RELEASE_METADATA + ) + ) + .to.emit(pluginRepo, 'VersionCreated') + .withArgs(2, 1, pluginSetupMock_2.address, BUILD_METADATA); - describe('getLatestVersion', async () => { - it('reverts if release does not exist', async () => { - await expect(pluginRepo['getLatestVersion(uint8)'](3)) - .to.be.revertedWithCustomError(pluginRepo, 'VersionHashDoesNotExist') - .withArgs(tagHash(3, 0)); + expect(await pluginRepo.latestRelease()).to.equal(2); }); - it('correctly returns the Version per release', async () => { - const func = pluginRepo['getLatestVersion(uint8)']; - - expect(await func(1)).to.deep.equal([ - [1, 2], - pluginSetup_R1_B2.address, - BUILD_METADATA_R1_B2, - ]); + it('succeeds if release already exists and release metadata is empty', async () => { + await pluginRepo.createVersion( + 1, + pluginSetupMock.address, + BUILD_METADATA, + RELEASE_METADATA + ); - expect(await func(2)).to.deep.equal([ - [2, 1], - pluginSetup_R2_B1.address, - BUILD_METADATA_R2_B1, - ]); + await expect( + pluginRepo.createVersion( + 1, + pluginSetupMock.address, + BUILD_METADATA, + '0x' + ) + ).to.not.emit(pluginRepo, 'ReleaseMetadataUpdated'); }); - it('reverts if plugin setup does not exist', async () => { - await expect(pluginRepo['getLatestVersion(address)'](ownerAddress)) - .to.be.revertedWithCustomError(pluginRepo, 'VersionHashDoesNotExist') - .withArgs( - '0x0000000000000000000000000000000000000000000000000000000000000000' - ); - }); + it('allows to create placeholder builds for the same release', async () => { + const PlaceholderSetup = new PlaceholderSetup__factory(signers[0]); + const placeholder1 = await PlaceholderSetup.deploy(); + const placeholder2 = await PlaceholderSetup.deploy(); - it('correctly returns the Version per plugin setup', async () => { - const func = pluginRepo['getLatestVersion(address)']; + // Release 1 + await expect( + pluginRepo.createVersion( + 1, + placeholder1.address, + ZERO_BYTES32, + ZERO_BYTES32 + ) + ) + .to.emit(pluginRepo, 'VersionCreated') + .withArgs(1, 1, placeholder1.address, ZERO_BYTES32); - expect(await func(pluginSetup_R1_B1.address)).to.deep.equal([ - [1, 1], - pluginSetup_R1_B1.address, - BUILD_METADATA_R1_B1, - ]); + await expect( + pluginRepo.createVersion( + 1, + placeholder1.address, + ZERO_BYTES32, + ZERO_BYTES32 + ) + ) + .to.emit(pluginRepo, 'VersionCreated') + .withArgs(1, 2, placeholder1.address, ZERO_BYTES32); - expect(await func(pluginSetup_R1_B2.address)).to.deep.equal([ - [1, 2], - pluginSetup_R1_B2.address, - BUILD_METADATA_R1_B2, - ]); + // Release 2 + await expect( + pluginRepo.createVersion( + 2, + placeholder2.address, + ZERO_BYTES32, + ZERO_BYTES32 + ) + ) + .to.emit(pluginRepo, 'VersionCreated') + .withArgs(2, 1, placeholder2.address, ZERO_BYTES32); - expect(await func(pluginSetup_R2_B1.address)).to.deep.equal([ - [2, 1], - pluginSetup_R2_B1.address, - BUILD_METADATA_R2_B1, - ]); + await expect( + pluginRepo.createVersion( + 2, + placeholder2.address, + ZERO_BYTES32, + ZERO_BYTES32 + ) + ) + .to.emit(pluginRepo, 'VersionCreated') + .withArgs(2, 2, placeholder2.address, ZERO_BYTES32); }); }); - describe('getVersion', async () => { - it('reverts if `Tag` does not exist', async () => { + describe('updateReleaseMetadata', async () => { + it('reverts if caller does not have permission', async () => { await expect( - pluginRepo['getVersion((uint8,uint16))']({release: 1, build: 3}) + pluginRepo + .connect(signers[2]) + .updateReleaseMetadata(1, RELEASE_METADATA) ) - .to.be.revertedWithCustomError(pluginRepo, 'VersionHashDoesNotExist') - .withArgs(tagHash(1, 3)); + .to.be.revertedWithCustomError(pluginRepo, 'Unauthorized') + .withArgs( + pluginRepo.address, + signers[2].address, + MAINTAINER_PERMISSION_ID + ); + }); + it('reverts if release is 0', async () => { + await expect( + pluginRepo.updateReleaseMetadata(0, emptyBytes) + ).to.be.revertedWithCustomError(pluginRepo, 'ReleaseZeroNotAllowed'); }); - it('correctly returns the version per `Tag`', async () => { - const func = pluginRepo['getVersion((uint8,uint16))']; - - expect(await func({release: 1, build: 1})).to.deep.equal([ - [1, 1], - pluginSetup_R1_B1.address, - BUILD_METADATA_R1_B1, - ]); - - expect(await func({release: 1, build: 2})).to.deep.equal([ - [1, 2], - pluginSetup_R1_B2.address, - BUILD_METADATA_R1_B2, - ]); + it('reverts if release does not exist', async () => { + await expect( + pluginRepo.updateReleaseMetadata(1, emptyBytes) + ).to.be.revertedWithCustomError(pluginRepo, 'ReleaseDoesNotExist'); + }); - expect(await func({release: 2, build: 1})).to.deep.equal([ - [2, 1], - pluginSetup_R2_B1.address, - BUILD_METADATA_R2_B1, - ]); + it('reverts if metadata length is 0', async () => { + await pluginRepo.createVersion( + 1, + pluginSetupMock.address, + BUILD_METADATA, + RELEASE_METADATA + ); + await expect( + pluginRepo.updateReleaseMetadata(1, '0x') + ).to.be.revertedWithCustomError(pluginRepo, 'EmptyReleaseMetadata'); }); - it('correctly returns the version per Tag hash', async () => { - const func = pluginRepo['getVersion(bytes32)']; + it('successfuly updates metadata for the release that already exists', async () => { + await pluginRepo.createVersion( + 1, + pluginSetupMock.address, + BUILD_METADATA, + RELEASE_METADATA + ); + await expect(pluginRepo.updateReleaseMetadata(1, '0x11')) + .to.emit(pluginRepo, 'ReleaseMetadataUpdated') + .withArgs(1, '0x11'); + }); + }); - expect(await func(tagHash(1, 1))).to.deep.equal([ - [1, 1], + describe('Different types of getVersions:', async () => { + // R - release, B - build + let pluginSetup_R1_B1: PluginUUPSUpgradeableSetupV1Mock; + let pluginSetup_R1_B2: PluginUUPSUpgradeableSetupV1Mock; + let pluginSetup_R2_B1: PluginUUPSUpgradeableSetupV1Mock; + let BUILD_METADATA_R1_B1 = BUILD_METADATA; + let BUILD_METADATA_R1_B2 = `${BUILD_METADATA}11`; + let BUILD_METADATA_R2_B1 = `${BUILD_METADATA}1111`; + + beforeEach(async () => { + pluginSetup_R1_B1 = pluginSetupMock; + pluginSetup_R1_B2 = await deployMockPluginSetup(signers[0]); + pluginSetup_R2_B1 = await deployMockPluginSetup(signers[0]); + + await pluginRepo.createVersion( + 1, pluginSetup_R1_B1.address, BUILD_METADATA_R1_B1, - ]); + RELEASE_METADATA + ); - expect(await func(tagHash(1, 2))).to.deep.equal([ - [1, 2], + await pluginRepo.createVersion( + 1, pluginSetup_R1_B2.address, BUILD_METADATA_R1_B2, - ]); + RELEASE_METADATA + ); - expect(await func(tagHash(2, 1))).to.deep.equal([ - [2, 1], + await pluginRepo.createVersion( + 2, pluginSetup_R2_B1.address, BUILD_METADATA_R2_B1, - ]); + RELEASE_METADATA + ); + }); + + describe('getLatestVersion', async () => { + it('reverts if release does not exist', async () => { + await expect(pluginRepo['getLatestVersion(uint8)'](3)) + .to.be.revertedWithCustomError( + pluginRepo, + 'VersionHashDoesNotExist' + ) + .withArgs(tagHash(3, 0)); + }); + + it('correctly returns the Version per release', async () => { + const func = pluginRepo['getLatestVersion(uint8)']; + + expect(await func(1)).to.deep.equal([ + [1, 2], + pluginSetup_R1_B2.address, + BUILD_METADATA_R1_B2, + ]); + + expect(await func(2)).to.deep.equal([ + [2, 1], + pluginSetup_R2_B1.address, + BUILD_METADATA_R2_B1, + ]); + }); + + it('reverts if plugin setup does not exist', async () => { + await expect(pluginRepo['getLatestVersion(address)'](ownerAddress)) + .to.be.revertedWithCustomError( + pluginRepo, + 'VersionHashDoesNotExist' + ) + .withArgs( + '0x0000000000000000000000000000000000000000000000000000000000000000' + ); + }); + + it('correctly returns the Version per plugin setup', async () => { + const func = pluginRepo['getLatestVersion(address)']; + + expect(await func(pluginSetup_R1_B1.address)).to.deep.equal([ + [1, 1], + pluginSetup_R1_B1.address, + BUILD_METADATA_R1_B1, + ]); + + expect(await func(pluginSetup_R1_B2.address)).to.deep.equal([ + [1, 2], + pluginSetup_R1_B2.address, + BUILD_METADATA_R1_B2, + ]); + + expect(await func(pluginSetup_R2_B1.address)).to.deep.equal([ + [2, 1], + pluginSetup_R2_B1.address, + BUILD_METADATA_R2_B1, + ]); + }); + }); + + describe('getVersion', async () => { + it('reverts if `Tag` does not exist', async () => { + await expect( + pluginRepo['getVersion((uint8,uint16))']({release: 1, build: 3}) + ) + .to.be.revertedWithCustomError( + pluginRepo, + 'VersionHashDoesNotExist' + ) + .withArgs(tagHash(1, 3)); + }); + + it('correctly returns the version per `Tag`', async () => { + const func = pluginRepo['getVersion((uint8,uint16))']; + + expect(await func({release: 1, build: 1})).to.deep.equal([ + [1, 1], + pluginSetup_R1_B1.address, + BUILD_METADATA_R1_B1, + ]); + + expect(await func({release: 1, build: 2})).to.deep.equal([ + [1, 2], + pluginSetup_R1_B2.address, + BUILD_METADATA_R1_B2, + ]); + + expect(await func({release: 2, build: 1})).to.deep.equal([ + [2, 1], + pluginSetup_R2_B1.address, + BUILD_METADATA_R2_B1, + ]); + }); + + it('correctly returns the version per Tag hash', async () => { + const func = pluginRepo['getVersion(bytes32)']; + + expect(await func(tagHash(1, 1))).to.deep.equal([ + [1, 1], + pluginSetup_R1_B1.address, + BUILD_METADATA_R1_B1, + ]); + + expect(await func(tagHash(1, 2))).to.deep.equal([ + [1, 2], + pluginSetup_R1_B2.address, + BUILD_METADATA_R1_B2, + ]); + + expect(await func(tagHash(2, 1))).to.deep.equal([ + [2, 1], + pluginSetup_R2_B1.address, + BUILD_METADATA_R2_B1, + ]); + }); }); }); }); diff --git a/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts b/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts index be90da591..c1460770c 100644 --- a/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts +++ b/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts @@ -1,7 +1,7 @@ import {expect} from 'chai'; import {ethers} from 'hardhat'; +import {ContractFactory} from 'ethers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; -import {deployWithProxy} from '../../../test-utils/proxy'; import { ENSSubdomainRegistrar, @@ -12,12 +12,15 @@ import { PublicResolver__factory, ENSSubdomainRegistrar__factory, } from '../../../../typechain'; +import {ENSSubdomainRegistrar__factory as ENSSubdomainRegistrar_V1_0_0__factory} from '../../../../typechain/@aragon/osx-v1.0.1/framework/utils/ens/ENSSubdomainRegistrar.sol'; + +import {deployWithProxy} from '../../../test-utils/proxy'; import {deployNewDAO} from '../../../test-utils/dao'; import {ensDomainHash, ensLabelHash} from '../../../../utils/ens'; import {OZ_ERRORS} from '../../../test-utils/error'; import {setupResolver} from '../../../test-utils/ens'; -import {shouldUpgradeCorrectly} from '../../../test-utils/uups-upgradeable'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; +import {upgradeManagedContract} from '../../../test-utils/uups-upgradeable'; const REGISTER_ENS_SUBDOMAIN_PERMISSION_ID = ethers.utils.id( 'REGISTER_ENS_SUBDOMAIN_PERMISSION' @@ -102,33 +105,6 @@ describe('ENSSubdomainRegistrar', function () { beforeEach(async function () { [ens, resolver, managingDao, registrar] = await setupENS(signers[0]); - - this.upgrade = { - contract: registrar, - dao: managingDao, - user: signers[8], - }; - }); - - describe('Upgrade', () => { - beforeEach(async function () { - await registerSubdomainHelper('test', '', signers[0], registrar.address); - this.upgrade = { - contract: registrar, - dao: managingDao, - user: signers[8], - }; - await registrar.initialize( - managingDao.address, - ens.address, - ensDomainHash('test') - ); - }); - - shouldUpgradeCorrectly( - UPGRADE_PERMISSIONS.UPGRADE_REGISTRAR_PERMISSION_ID, - 'DaoUnauthorized' - ); }); describe('Check the initial ENS state', async () => { @@ -156,7 +132,7 @@ describe('ENSSubdomainRegistrar', function () { await registrar .connect(signers[0]) .initialize(managingDao.address, ens.address, ensDomainHash('test')) - ); + ).to.not.be.revertedWithCustomError(registrar, 'InvalidResolver'); }); postInitializationTests(); @@ -305,6 +281,40 @@ describe('ENSSubdomainRegistrar', function () { expectedReverts(); }); + describe('Upgrades', () => { + let legacyContractFactory: ContractFactory; + let currentContractFactory: ContractFactory; + + before(() => { + currentContractFactory = new ENSSubdomainRegistrar__factory(signers[0]); + }); + + beforeEach(async () => { + await registerSubdomainHelper('test', '', signers[0], registrar.address); + }); + + it('from v1.0.0', async () => { + legacyContractFactory = new ENSSubdomainRegistrar_V1_0_0__factory( + signers[0] + ); + + await upgradeManagedContract( + signers[0], + signers[1], + managingDao, + { + managingDao: managingDao.address, + ens: ens.address, + parentDomain: ensDomainHash('test'), + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_REGISTRAR_PERMISSION_ID + ); + }); + }); + function expectedReverts() { it('reverts during initialization if node does not have a valid resolver', async () => { await expect( diff --git a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts index 6ff1c4f83..53f3c7e36 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts @@ -1,5 +1,6 @@ import {expect} from 'chai'; import {ethers} from 'hardhat'; +import {ContractFactory} from 'ethers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import { @@ -14,6 +15,13 @@ import { IPlugin__factory, IProposal__factory, } from '../../../../../typechain'; +import {AddresslistVoting__factory as AddresslistVoting_V1_0_0__factory} from '../../../../../typechain/@aragon/osx-v1.0.1/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol'; +import { + ProposalCreatedEvent, + ProposalExecutedEvent, +} from '../../../../../typechain/AddresslistVoting'; +import {majorityVotingBaseInterface} from '../majority-voting'; + import { findEvent, findEventTopicLog, @@ -37,15 +45,10 @@ import { } from '../../../../test-utils/voting'; import {deployNewDAO} from '../../../../test-utils/dao'; import {OZ_ERRORS} from '../../../../test-utils/error'; -import {shouldUpgradeCorrectly} from '../../../../test-utils/uups-upgradeable'; import {UPGRADE_PERMISSIONS} from '../../../../test-utils/permissions'; import {deployWithProxy} from '../../../../test-utils/proxy'; import {getInterfaceID} from '../../../../test-utils/interfaces'; -import {majorityVotingBaseInterface} from '../majority-voting'; -import { - ProposalCreatedEvent, - ProposalExecutedEvent, -} from '../../../../../typechain/AddresslistVoting'; +import {upgradeManagedContract} from '../../../../test-utils/uups-upgradeable'; export const addresslistVotingInterface = new ethers.utils.Interface([ 'function initialize(address,tuple(uint8,uint32,uint32,uint64,uint256),address[])', @@ -109,28 +112,6 @@ describe('AddresslistVoting', function () { signers[0].address, ethers.utils.id('UPDATE_ADDRESSES_PERMISSION') ); - - this.upgrade = { - contract: voting, - dao: dao, - user: signers[8], - }; - }); - - describe('Upgrade', () => { - beforeEach(async function () { - this.upgrade = { - contract: voting, - dao: dao, - user: signers[8], - }; - await voting.initialize(dao.address, votingSettings, []); - }); - - shouldUpgradeCorrectly( - UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, - 'DaoUnauthorized' - ); }); describe('initialize: ', async () => { @@ -143,6 +124,34 @@ describe('AddresslistVoting', function () { }); }); + describe('Upgrades', () => { + let legacyContractFactory: ContractFactory; + let currentContractFactory: ContractFactory; + + before(() => { + currentContractFactory = new AddresslistVoting__factory(signers[0]); + }); + + it('from v1.0.0', async () => { + legacyContractFactory = new AddresslistVoting_V1_0_0__factory(signers[0]); + + await upgradeManagedContract( + signers[0], + signers[1], + dao, + { + dao: dao.address, + votingSettings: votingSettings, + members: [signers[0].address, signers[1].address], + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + ); + }); + }); + describe('plugin interface: ', async () => { it('does not support the empty interface', async () => { expect(await voting.supportsInterface('0xffffffff')).to.be.false; diff --git a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts index c048bb480..4ec670422 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts @@ -1,6 +1,6 @@ import {expect} from 'chai'; import {ethers} from 'hardhat'; -import {BigNumber} from 'ethers'; +import {BigNumber, ContractFactory} from 'ethers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import { @@ -16,6 +16,12 @@ import { TokenVoting, TokenVoting__factory, } from '../../../../../typechain'; +import {TokenVoting__factory as TokenVoting_V1_0_0__factory} from '../../../../../typechain/@aragon/osx-v1.0.1/plugins/governance/majority-voting/token/TokenVoting.sol'; +import { + ProposalCreatedEvent, + ProposalExecutedEvent, +} from '../../../../../typechain/TokenVoting'; + import { findEvent, findEventTopicLog, @@ -40,15 +46,11 @@ import { } from '../../../../test-utils/voting'; import {deployNewDAO} from '../../../../test-utils/dao'; import {OZ_ERRORS} from '../../../../test-utils/error'; -import {shouldUpgradeCorrectly} from '../../../../test-utils/uups-upgradeable'; -import {UPGRADE_PERMISSIONS} from '../../../../test-utils/permissions'; import {deployWithProxy} from '../../../../test-utils/proxy'; import {getInterfaceID} from '../../../../test-utils/interfaces'; +import {UPGRADE_PERMISSIONS} from '../../../../test-utils/permissions'; +import {upgradeManagedContract} from '../../../../test-utils/uups-upgradeable'; import {majorityVotingBaseInterface} from '../majority-voting'; -import { - ProposalCreatedEvent, - ProposalExecutedEvent, -} from '../../../../../typechain/TokenVoting'; export const tokenVotingInterface = new ethers.utils.Interface([ 'function initialize(address,tuple(uint8,uint32,uint32,uint64,uint256),address)', @@ -122,26 +124,6 @@ describe('TokenVoting', function () { ); }); - describe('Upgrade', () => { - beforeEach(async function () { - this.upgrade = { - contract: voting, - dao: dao, - user: signers[8], - }; - await voting.initialize( - dao.address, - votingSettings, - governanceErc20Mock.address - ); - }); - - shouldUpgradeCorrectly( - UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, - 'DaoUnauthorized' - ); - }); - async function setBalances( balances: {receiver: string; amount: number | BigNumber}[] ) { @@ -210,6 +192,34 @@ describe('TokenVoting', function () { }); }); + describe('Upgrades', () => { + let legacyContractFactory: ContractFactory; + let currentContractFactory: ContractFactory; + + before(() => { + currentContractFactory = new TokenVoting__factory(signers[0]); + }); + + it('from v1.0.0', async () => { + legacyContractFactory = new TokenVoting_V1_0_0__factory(signers[0]); + + await upgradeManagedContract( + signers[0], + signers[1], + dao, + { + dao: dao.address, + votingSettings: votingSettings, + token: governanceErc20Mock.address, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + ); + }); + }); + describe('plugin interface: ', async () => { it('does not support the empty interface', async () => { expect(await voting.supportsInterface('0xffffffff')).to.be.false; diff --git a/packages/contracts/test/plugins/governance/multisig/multisig.ts b/packages/contracts/test/plugins/governance/multisig/multisig.ts index 127829571..4bf2e637c 100644 --- a/packages/contracts/test/plugins/governance/multisig/multisig.ts +++ b/packages/contracts/test/plugins/governance/multisig/multisig.ts @@ -1,6 +1,6 @@ import {expect} from 'chai'; import {ethers} from 'hardhat'; -import {Contract} from 'ethers'; +import {Contract, ContractFactory} from 'ethers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import { @@ -15,6 +15,13 @@ import { Multisig, Multisig__factory, } from '../../../../typechain'; +import {Multisig__factory as Multisig_V1_0_0__factory} from '../../../../typechain/@aragon/osx-v1.0.1/plugins/governance/multisig/Multisig.sol'; +import { + ApprovedEvent, + ProposalExecutedEvent, +} from '../../../../typechain/Multisig'; +import {ProposalCreatedEvent} from '../../../../typechain/IProposal'; + import { findEvent, findEventTopicLog, @@ -32,16 +39,10 @@ import { timestampIn, toBytes32, } from '../../../test-utils/voting'; -import {shouldUpgradeCorrectly} from '../../../test-utils/uups-upgradeable'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; import {deployWithProxy} from '../../../test-utils/proxy'; import {getInterfaceID} from '../../../test-utils/interfaces'; -import { - ApprovedEvent, - ProposalExecutedEvent, -} from '../../../../typechain/Multisig'; -import {ExecutedEvent} from '../../../../typechain/DAO'; -import {ProposalCreatedEvent} from '../../../../typechain/IProposal'; +import {upgradeManagedContract} from '../../../test-utils/uups-upgradeable'; export const multisigInterface = new ethers.utils.Interface([ 'function initialize(address,address[],tuple(bool,uint16))', @@ -115,26 +116,6 @@ describe('Multisig', function () { ); }); - describe('Upgrade', () => { - beforeEach(async function () { - this.upgrade = { - contract: multisig, - dao: dao, - user: signers[8], - }; - await multisig.initialize( - dao.address, - signers.slice(0, 5).map(s => s.address), - multisigSettings - ); - }); - - shouldUpgradeCorrectly( - UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, - 'DaoUnauthorized' - ); - }); - describe('initialize:', async () => { it('reverts if trying to re-initialize', async () => { await multisig.initialize( @@ -210,6 +191,34 @@ describe('Multisig', function () { }); }); + describe('Upgrades', () => { + let legacyContractFactory: ContractFactory; + let currentContractFactory: ContractFactory; + + before(() => { + currentContractFactory = new Multisig__factory(signers[0]); + }); + + it('from v1.0.0', async () => { + legacyContractFactory = new Multisig_V1_0_0__factory(signers[0]); + + await upgradeManagedContract( + signers[0], + signers[1], + dao, + { + dao: dao.address, + members: [signers[0].address, signers[1].address, signers[2].address], + multisigSettings: multisigSettings, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + ); + }); + }); + describe('plugin interface: ', async () => { it('does not support the empty interface', async () => { expect(await multisig.supportsInterface('0xffffffff')).to.be.false; diff --git a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts index 821a8ab53..02634380f 100644 --- a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts +++ b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts @@ -19,7 +19,6 @@ import {deployWithProxy} from '../../../test-utils/proxy'; import BalanceTree from './src/balance-tree'; import {deployNewDAO} from '../../../test-utils/dao'; import {getInterfaceID} from '../../../test-utils/interfaces'; -import {shouldUpgradeCorrectly} from '../../../test-utils/uups-upgradeable'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; const ZERO_BYTES32 = `0x${`0`.repeat(64)}`; @@ -71,22 +70,6 @@ describe('MerkleDistributor', function () { }); }); - describe('Upgrade', () => { - beforeEach(async function () { - this.upgrade = { - contract: distributor, - dao: dao, - user: signers[8], - }; - await distributor.initialize(dao.address, token.address, ZERO_BYTES32); - }); - - shouldUpgradeCorrectly( - UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, - 'DaoUnauthorized' - ); - }); - describe('general', () => { beforeEach(async function () { await distributor.initialize(dao.address, token.address, ZERO_BYTES32); diff --git a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts index 1dea27e7b..a38cb7d01 100644 --- a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts +++ b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts @@ -21,7 +21,6 @@ import BalanceTree from './src/balance-tree'; import {deployNewDAO} from '../../../test-utils/dao'; import {deployWithProxy} from '../../../test-utils/proxy'; import {getInterfaceID} from '../../../test-utils/interfaces'; -import {shouldUpgradeCorrectly} from '../../../test-utils/uups-upgradeable'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; const MERKLE_MINT_PERMISSION_ID = ethers.utils.id('MERKLE_MINT_PERMISSION'); @@ -79,19 +78,8 @@ describe('MerkleMinter', function () { MERKLE_MINT_PERMISSION_ID ); await managingDao.grant(token.address, minter.address, MINT_PERMISSION_ID); - - this.upgrade = { - contract: minter, - dao: managingDao, - user: signers[8], - }; }); - shouldUpgradeCorrectly( - UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, - 'DaoUnauthorized' - ); - describe('plugin interface: ', async () => { it('does not support the empty interface', async () => { expect(await minter.supportsInterface('0xffffffff')).to.be.false; diff --git a/packages/contracts/test/test-utils/uups-upgradeable.ts b/packages/contracts/test/test-utils/uups-upgradeable.ts index 8be72d25d..0056620ea 100644 --- a/packages/contracts/test/test-utils/uups-upgradeable.ts +++ b/packages/contracts/test/test-utils/uups-upgradeable.ts @@ -1,96 +1,99 @@ import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; -import {Contract} from 'ethers'; -import {defaultAbiCoder} from 'ethers/lib/utils'; -import {ethers} from 'hardhat'; -import {PluginUUPSUpgradeableV1Mock__factory} from '../../typechain'; +import {ContractFactory} from 'ethers'; +import {upgrades} from 'hardhat'; +import {DAO, PluginRepo} from '../../typechain'; // See https://eips.ethereum.org/EIPS/eip-1967 export const IMPLEMENTATION_SLOT = '0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc'; // bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1) -/// Used as a common test suite to test upgradeability of the contracts. -/// Presumes that `upgrade` object is set on `this` inside the actual test file. -/// this.upgrade consists of: -/// contract - address of the contract on which it tests if `upgradeTo` works as intended. -/// dao - dao contact that the contract belongs to. -/// user - ethers user object. Presumed that it doesn't have permission to call `upgradeTo`. -export function shouldUpgradeCorrectly( - upgradePermissionId: string, - upgradeRevertPermissionMessage: string +export async function upgradeManagedContract( + deployer: SignerWithAddress, + upgrader: SignerWithAddress, + managingDao: DAO, + initArgs: any, + initializerName: string, + from: ContractFactory, + to: ContractFactory, + upgradePermissionId: string ) { - let uupsCompatibleBase: string; + // Deploy the proxy + const proxy = await upgrades.deployProxy( + from.connect(deployer), + Object.values(initArgs), + { + kind: 'uups', + initializer: initializerName, + unsafeAllow: ['constructor'], + constructorArgs: [], + } + ); - function DaoUnauthorizedRevertArgs( - contract: Contract, - user: SignerWithAddress, - dao: Contract - ) { - return [dao.address, contract.address, user.address, upgradePermissionId]; - } + await expect( + upgrades.upgradeProxy(proxy.address, to.connect(upgrader), { + unsafeAllow: ['constructor'], + constructorArgs: [], + }) + ) + .to.be.revertedWithCustomError(proxy, 'DaoUnauthorized') + .withArgs( + managingDao.address, + proxy.address, + upgrader.address, + upgradePermissionId + ); - function UnauthorizedRevertArgs(dao: Contract, user: SignerWithAddress) { - return [dao.address, user.address, upgradePermissionId]; - } + // Grant the upgrade permission + await managingDao + .connect(deployer) + .grant(proxy.address, upgrader.address, upgradePermissionId); - describe('UUPS Upgradeability Test', async () => { - before(async () => { - const signers = await ethers.getSigners(); - const factory = new PluginUUPSUpgradeableV1Mock__factory(signers[0]); - uupsCompatibleBase = (await factory.deploy()).address; - }); + // Upgrade the proxy + await upgrades.upgradeProxy(proxy.address, to.connect(upgrader), { + unsafeAllow: ['constructor'], + constructorArgs: [], + }); +} - it('reverts if user without permission tries to upgrade', async function () { - const {user, contract, dao} = this.upgrade; - const connect = contract.connect(user); - const tx1 = connect.upgradeTo(ethers.constants.AddressZero); - const tx2 = connect.upgradeToAndCall(ethers.constants.AddressZero, '0x'); - if (upgradeRevertPermissionMessage === 'DaoUnauthorized') { - await expect(tx1) - .to.be.revertedWithCustomError( - contract, - upgradeRevertPermissionMessage - ) - .withArgs(...DaoUnauthorizedRevertArgs(contract, user, dao)); - await expect(tx2) - .to.be.revertedWithCustomError( - contract, - upgradeRevertPermissionMessage - ) - .withArgs(...DaoUnauthorizedRevertArgs(contract, user, dao)); - } else { - await expect(tx1) - .to.be.revertedWithCustomError( - contract, - upgradeRevertPermissionMessage - ) - .withArgs(...UnauthorizedRevertArgs(dao, user)); - await expect(tx2) - .to.be.revertedWithCustomError( - contract, - upgradeRevertPermissionMessage - ) - .withArgs(...UnauthorizedRevertArgs(dao, user)); - } - }); +export async function upgradeManagingContract( + deployer: SignerWithAddress, + upgrader: SignerWithAddress, + initArgs: any, + initializerName: string, + from: ContractFactory, + to: ContractFactory, + upgradePermissionId: string +) { + // Deploy the proxy + const proxy = await upgrades.deployProxy( + from.connect(deployer), + Object.values(initArgs), + { + kind: 'uups', + initializer: initializerName, + unsafeAllow: ['constructor'], + constructorArgs: [], + } + ); - it('updates correctly to new implementation', async function () { - const {user, contract, dao} = this.upgrade; - await dao.grant(contract.address, user.address, upgradePermissionId); - const connect = contract.connect(user); + await expect( + upgrades.upgradeProxy(proxy.address, to.connect(upgrader), { + unsafeAllow: ['constructor'], + constructorArgs: [], + }) + ) + .to.be.revertedWithCustomError(proxy, 'Unauthorized') + .withArgs(proxy.address, upgrader.address, upgradePermissionId); - // Check the event. - await expect(connect.upgradeTo(uupsCompatibleBase)) - .to.emit(contract, 'Upgraded') - .withArgs(uupsCompatibleBase); + // Grant the upgrade permission + await proxy + .connect(deployer) + .grant(proxy.address, upgrader.address, upgradePermissionId); - // Check the storage slot. - const encoded = await ethers.provider.getStorageAt( - contract.address, - IMPLEMENTATION_SLOT - ); - const implementation = defaultAbiCoder.decode(['address'], encoded)[0]; - expect(implementation).to.equal(uupsCompatibleBase); - }); + // Upgrade the proxy + await upgrades.upgradeProxy(proxy.address, to.connect(upgrader), { + unsafeAllow: ['constructor'], + constructorArgs: [], }); } diff --git a/packages/contracts/utils/storage.ts b/packages/contracts/utils/storage.ts index d1f0af135..9ef36ff2f 100644 --- a/packages/contracts/utils/storage.ts +++ b/packages/contracts/utils/storage.ts @@ -3,7 +3,7 @@ import {defaultAbiCoder} from 'ethers/lib/utils'; import {IMPLEMENTATION_SLOT} from '../test/test-utils/uups-upgradeable'; -export function readImplementationValuesFromSlot( +export async function readImplementationValuesFromSlot( contractAddresses: string[] ): Promise { return Promise.all( @@ -13,7 +13,7 @@ export function readImplementationValuesFromSlot( ); } -export function readImplementationValueFromSlot( +export async function readImplementationValueFromSlot( contractAddress: string ): Promise { return ethers.provider From 19ac7c31f4a029c65fe7219b106d0fc5bdb008f6 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Wed, 19 Jul 2023 18:13:37 +0200 Subject: [PATCH 12/31] feature: merkle minter and distributor tests --- packages/contracts/src/test/Migration.sol | 3 ++ .../token/distribution/merkle-distributor.ts | 37 ++++++++++++++++++- .../token/distribution/merkle-minter.ts | 37 ++++++++++++++++++- 3 files changed, 73 insertions(+), 4 deletions(-) diff --git a/packages/contracts/src/test/Migration.sol b/packages/contracts/src/test/Migration.sol index f70702516..d5fdae2b2 100644 --- a/packages/contracts/src/test/Migration.sol +++ b/packages/contracts/src/test/Migration.sol @@ -28,3 +28,6 @@ import {ENSSubdomainRegistrar as ENSSubdomainRegistrar_v1_0_0} from "@aragon/osx import {TokenVoting as TokenVoting_v1_0_0} from "@aragon/osx-v1.0.1/plugins/governance/majority-voting/token/TokenVoting.sol"; import {AddresslistVoting as AddresslistVoting_v1_0_0} from "@aragon/osx-v1.0.1/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol"; import {Multisig as Multisig_v1_0_0} from "@aragon/osx-v1.0.1/plugins/governance/multisig/Multisig.sol"; + +import {MerkleMinter as MerkleMinter_v1_0_0} from "@aragon/osx-v1.0.1/plugins/token/MerkleMinter.sol"; +import {MerkleDistributor as MerkleDistributor_v1_0_0} from "@aragon/osx-v1.0.1/plugins/token/MerkleDistributor.sol"; diff --git a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts index 02634380f..47dd233db 100644 --- a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts +++ b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts @@ -2,7 +2,7 @@ import {expect} from 'chai'; import {ethers} from 'hardhat'; -import {BigNumber} from 'ethers'; +import {BigNumber, ContractFactory} from 'ethers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import { @@ -15,11 +15,14 @@ import { TestERC20__factory, MerkleDistributor__factory, } from '../../../../typechain'; +import {MerkleDistributor__factory as MerkleDistributor_V1_0_0__factory} from '../../../../typechain/@aragon/osx-v1.0.1/plugins/token/MerkleDistributor.sol'; + import {deployWithProxy} from '../../../test-utils/proxy'; import BalanceTree from './src/balance-tree'; import {deployNewDAO} from '../../../test-utils/dao'; import {getInterfaceID} from '../../../test-utils/interfaces'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; +import {upgradeManagedContract} from '../../../test-utils/uups-upgradeable'; const ZERO_BYTES32 = `0x${`0`.repeat(64)}`; @@ -31,11 +34,13 @@ describe('MerkleDistributor', function () { let wallet0: string; let wallet1: string; - beforeEach(async function () { + before(async function () { signers = await ethers.getSigners(); wallet0 = await signers[0].getAddress(); wallet1 = await signers[1].getAddress(); + }); + beforeEach(async function () { // create a DAO dao = await deployNewDAO(signers[0]); @@ -70,6 +75,34 @@ describe('MerkleDistributor', function () { }); }); + describe('Upgrades', () => { + let legacyContractFactory: ContractFactory; + let currentContractFactory: ContractFactory; + + before(() => { + currentContractFactory = new MerkleDistributor__factory(signers[0]); + }); + + it('from v1.0.0', async () => { + legacyContractFactory = new MerkleDistributor_V1_0_0__factory(signers[0]); + + await upgradeManagedContract( + signers[0], + signers[1], + dao, + { + dao: dao.address, + token: token.address, + merkleRoot: ZERO_BYTES32, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + ); + }); + }); + describe('general', () => { beforeEach(async function () { await distributor.initialize(dao.address, token.address, ZERO_BYTES32); diff --git a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts index a38cb7d01..1e04ef55a 100644 --- a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts +++ b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts @@ -2,7 +2,7 @@ import {expect} from 'chai'; import {ethers} from 'hardhat'; -import {BigNumber} from 'ethers'; +import {BigNumber, ContractFactory} from 'ethers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import { @@ -17,11 +17,14 @@ import { MerkleDistributor__factory, GovernanceERC20__factory, } from '../../../../typechain'; +import {MerkleMinter__factory as MerkleMinter_V1_0_0__factory} from '../../../../typechain/@aragon/osx-v1.0.1/plugins/token/MerkleMinter.sol'; + import BalanceTree from './src/balance-tree'; import {deployNewDAO} from '../../../test-utils/dao'; import {deployWithProxy} from '../../../test-utils/proxy'; import {getInterfaceID} from '../../../test-utils/interfaces'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; +import {upgradeManagedContract} from '../../../test-utils/uups-upgradeable'; const MERKLE_MINT_PERMISSION_ID = ethers.utils.id('MERKLE_MINT_PERMISSION'); const MINT_PERMISSION_ID = ethers.utils.id('MINT_PERMISSION'); @@ -38,10 +41,12 @@ describe('MerkleMinter', function () { let merkleRoot: string; let totalAmount: BigNumber; - beforeEach(async function () { + before(async function () { signers = await ethers.getSigners(); ownerAddress = await signers[0].getAddress(); + }); + beforeEach(async function () { const amount0 = BigNumber.from(100); const amount1 = BigNumber.from(101); @@ -80,6 +85,34 @@ describe('MerkleMinter', function () { await managingDao.grant(token.address, minter.address, MINT_PERMISSION_ID); }); + describe('Upgrades', () => { + let legacyContractFactory: ContractFactory; + let currentContractFactory: ContractFactory; + + before(() => { + currentContractFactory = new MerkleMinter__factory(signers[0]); + }); + + it('from v1.0.0', async () => { + legacyContractFactory = new MerkleMinter_V1_0_0__factory(signers[0]); + + await upgradeManagedContract( + signers[0], + signers[1], + managingDao, + { + dao: managingDao.address, + token: token.address, + merkleDistributor: distributorBase.address, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + ); + }); + }); + describe('plugin interface: ', async () => { it('does not support the empty interface', async () => { expect(await minter.supportsInterface('0xffffffff')).to.be.false; From 2d4609ac636754cdd6e10570baac0084b8ed97d1 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Thu, 20 Jul 2023 09:37:29 +0200 Subject: [PATCH 13/31] fix: use explicit artifacts for managing DAO tests --- .../contracts/test/deploy/managing-dao.ts | 113 ++++++++++-------- 1 file changed, 60 insertions(+), 53 deletions(-) diff --git a/packages/contracts/test/deploy/managing-dao.ts b/packages/contracts/test/deploy/managing-dao.ts index 278fc2431..c22cad195 100644 --- a/packages/contracts/test/deploy/managing-dao.ts +++ b/packages/contracts/test/deploy/managing-dao.ts @@ -17,6 +17,11 @@ import { } from '../../typechain'; import daoArtifactData from '../../artifacts/src/core/dao/DAO.sol/DAO.json'; +import daoRegistryArtifactData from '../../artifacts/@aragon/osx-v1.0.1/framework/dao/DAORegistry.sol/DAORegistry.json'; +import pluginRepoRegistryArtifactData from '../../artifacts/@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepoRegistry.sol/PluginRepoRegistry.json'; +import pluginRepoArtifactData from '../../artifacts/@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepo.sol/PluginRepo.json'; +import ensSubdomainRegistrarArtifactData from '../../artifacts/@aragon/osx-v1.0.1/framework/utils/ens/ENSSubdomainRegistrar.sol/ENSSubdomainRegistrar.json'; + import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {initializeDeploymentFixture} from '../test-utils/fixture'; @@ -24,7 +29,7 @@ async function deployAll() { await initializeDeploymentFixture('New'); } -describe('Managing DAO', function () { +describes('Managing DAO', function () { let signers: SignerWithAddress[]; let ownerAddress: string; let managingDaoDeployment: Deployment; @@ -157,19 +162,18 @@ describe('Managing DAO', function () { it('Should be able to upgrade `DaoRegistry`', async function () { // deploy a new implementation. - await deployments.deploy('DAORegistryV2', { - contract: 'DAORegistry', - from: ownerAddress, - args: [], - log: true, - }); - - const daoRegistryV2Deployment: Deployment = await deployments.get( - 'DAORegistryV2' + const daoRegistry_v1_0_0_Deployment = await deployments.deploy( + 'DAORegistry_v1_0_0', + { + contract: daoRegistryArtifactData, + from: ownerAddress, + args: [], + log: true, + } ); // make sure new `DAORegistryV2` deployment is just an implementation and not a proxy - expect(daoRegistryV2Deployment.implementation).to.be.equal(undefined); + expect(daoRegistry_v1_0_0_Deployment.implementation).to.be.equal(undefined); // check new implementation is deferent from the one on the `DaoRegistry`. // read from slot @@ -177,12 +181,14 @@ describe('Managing DAO', function () { await readImplementationValuesFromSlot([daoRegistry.address]) )[0]; - expect(daoRegistryV2Deployment.address).not.equal(implementationAddress); + expect(daoRegistry_v1_0_0_Deployment.address).not.equal( + implementationAddress + ); // create proposal to upgrade to new implementation await createUpgradeProposal( [daoRegistry.address], - daoRegistryV2Deployment.address + daoRegistry_v1_0_0_Deployment.address ); // re-read from slot @@ -190,24 +196,25 @@ describe('Managing DAO', function () { await readImplementationValuesFromSlot([daoRegistry.address]) )[0]; - expect(daoRegistryV2Deployment.address).to.be.equal(implementationAddress); + expect(daoRegistry_v1_0_0_Deployment.address).to.be.equal( + implementationAddress + ); }); it('Should be able to upgrade `PluginRepoRegistry`', async function () { // deploy a new implementation. - await deployments.deploy('PluginRepoRegistryV2', { - contract: 'PluginRepoRegistry', - from: ownerAddress, - args: [], - log: true, - }); - - const pluginRepoRegistryV2Deployment: Deployment = await deployments.get( - 'PluginRepoRegistryV2' + const pluginRepoRegistry_v1_0_0_Deployment = await deployments.deploy( + 'PluginRepoRegistry_v1_0_0', + { + contract: pluginRepoRegistryArtifactData, + from: ownerAddress, + args: [], + log: true, + } ); // make sure new `PluginRepoRegistryV2` deployment is just an implementation and not a proxy - expect(pluginRepoRegistryV2Deployment.implementation).to.be.equal( + expect(pluginRepoRegistry_v1_0_0_Deployment.implementation).to.be.equal( undefined ); @@ -217,14 +224,14 @@ describe('Managing DAO', function () { await readImplementationValuesFromSlot([pluginRepoRegistry.address]) )[0]; - expect(pluginRepoRegistryV2Deployment.address).not.equal( + expect(pluginRepoRegistry_v1_0_0_Deployment.address).not.equal( implementationAddress ); // create proposal to upgrade to new implementation await createUpgradeProposal( [pluginRepoRegistry.address], - pluginRepoRegistryV2Deployment.address + pluginRepoRegistry_v1_0_0_Deployment.address ); // re-read from slot @@ -232,26 +239,25 @@ describe('Managing DAO', function () { await readImplementationValuesFromSlot([pluginRepoRegistry.address]) )[0]; - expect(pluginRepoRegistryV2Deployment.address).to.be.equal( + expect(pluginRepoRegistry_v1_0_0_Deployment.address).to.be.equal( implementationAddress ); }); it('Should be able to upgrade `ENSSubdomainRegistrar`', async function () { // deploy a new implementation. - await deployments.deploy('ENSSubdomainRegistrarV2', { - contract: 'ENSSubdomainRegistrar', - from: ownerAddress, - args: [], - log: true, - }); - - const ensSubdomainRegistrarV2Deployment: Deployment = await deployments.get( - 'ENSSubdomainRegistrarV2' + const ensSubdomainRegistrar_v1_0_0_Deployment = await deployments.deploy( + 'ENSSubdomainRegistrar_v1_0_0', + { + contract: ensSubdomainRegistrarArtifactData, + from: ownerAddress, + args: [], + log: true, + } ); // make sure new `ENSSubdomainRegistrarV2` deployment is just an implementation and not a proxy - expect(ensSubdomainRegistrarV2Deployment.implementation).to.be.equal( + expect(ensSubdomainRegistrar_v1_0_0_Deployment.implementation).to.be.equal( undefined ); @@ -264,7 +270,7 @@ describe('Managing DAO', function () { for (let index = 0; index < implementationValues.length; index++) { const implementationAddress = implementationValues[index]; - expect(ensSubdomainRegistrarV2Deployment.address).not.equal( + expect(ensSubdomainRegistrar_v1_0_0_Deployment.address).not.equal( implementationAddress ); } @@ -272,7 +278,7 @@ describe('Managing DAO', function () { // create proposal to upgrade to new implementation await createUpgradeProposal( ensSubdomainRegistrars.map(ensSR => ensSR.address), - ensSubdomainRegistrarV2Deployment.address + ensSubdomainRegistrar_v1_0_0_Deployment.address ); // re-read from slot @@ -283,7 +289,7 @@ describe('Managing DAO', function () { for (let index = 0; index < implementationValues.length; index++) { const implementationAddress = implementationValues[index]; - expect(ensSubdomainRegistrarV2Deployment.address).to.be.equal( + expect(ensSubdomainRegistrar_v1_0_0_Deployment.address).to.be.equal( implementationAddress ); } @@ -291,19 +297,18 @@ describe('Managing DAO', function () { it('Should be able to upgrade `PluginRepo`s', async function () { // deploy a new implementation. - await deployments.deploy('PluginRepoV2', { - contract: 'PluginRepo', - from: ownerAddress, - args: [], - log: true, - }); - - const pluginRepoV2Deployment: Deployment = await deployments.get( - 'PluginRepoV2' + const PluginRepo_v1_0_0_Deployment = await deployments.deploy( + 'PluginRepo_v1_0_0', + { + contract: pluginRepoArtifactData, + from: ownerAddress, + args: [], + log: true, + } ); // make sure new `PluginRepoV2` deployment is just an implementation and not a proxy - expect(pluginRepoV2Deployment.implementation).to.be.equal(undefined); + expect(PluginRepo_v1_0_0_Deployment.implementation).to.be.equal(undefined); // check new implementation is deferent from the one on the `DaoRegistry`. // read from slot @@ -316,7 +321,7 @@ describe('Managing DAO', function () { for (let index = 0; index < implementationValues.length; index++) { const implementationAddress = implementationValues[index]; - expect(pluginRepoV2Deployment.address).to.not.equal( + expect(PluginRepo_v1_0_0_Deployment.address).to.not.equal( implementationAddress ); } @@ -324,7 +329,7 @@ describe('Managing DAO', function () { // create proposal to upgrade to new implementation await createUpgradeProposal( Object.values(hre.aragonPluginRepos), - pluginRepoV2Deployment.address + PluginRepo_v1_0_0_Deployment.address ); // re-read from slot @@ -337,7 +342,9 @@ describe('Managing DAO', function () { for (let index = 0; index < implementationValues.length; index++) { const implementationAddress = implementationValues[index]; - expect(pluginRepoV2Deployment.address).to.be.equal(implementationAddress); + expect(PluginRepo_v1_0_0_Deployment.address).to.be.equal( + implementationAddress + ); } }); }); From 934137805e7d279a59bede01a70ada7e4ef08c9e Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Thu, 20 Jul 2023 09:41:27 +0200 Subject: [PATCH 14/31] fix: typo --- packages/contracts/test/deploy/managing-dao.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts/test/deploy/managing-dao.ts b/packages/contracts/test/deploy/managing-dao.ts index c22cad195..798188f2d 100644 --- a/packages/contracts/test/deploy/managing-dao.ts +++ b/packages/contracts/test/deploy/managing-dao.ts @@ -29,7 +29,7 @@ async function deployAll() { await initializeDeploymentFixture('New'); } -describes('Managing DAO', function () { +describe('Managing DAO', function () { let signers: SignerWithAddress[]; let ownerAddress: string; let managingDaoDeployment: Deployment; From 4b144c1d85d99852916145c15cd4764ad52925a8 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Thu, 20 Jul 2023 10:00:43 +0200 Subject: [PATCH 15/31] feature: improved names and comments --- packages/contracts/test/core/dao/dao.ts | 4 ++-- packages/contracts/test/framework/dao/dao-registry.ts | 4 ++-- .../test/framework/plugin/plugin-repo-registry.ts | 4 ++-- packages/contracts/test/framework/plugin/plugin-repo.ts | 4 ++-- .../test/framework/utils/ens/ens-subdomain-registry.ts | 4 ++-- .../majority-voting/addresslist/addresslist-voting.ts | 4 ++-- .../governance/majority-voting/token/token-voting.ts | 4 ++-- .../test/plugins/governance/multisig/multisig.ts | 4 ++-- .../test/plugins/token/distribution/merkle-distributor.ts | 4 ++-- .../test/plugins/token/distribution/merkle-minter.ts | 4 ++-- packages/contracts/test/test-utils/uups-upgradeable.ts | 8 ++++++-- 11 files changed, 26 insertions(+), 22 deletions(-) diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts index b8bcbcd2f..072d12592 100644 --- a/packages/contracts/test/core/dao/dao.ts +++ b/packages/contracts/test/core/dao/dao.ts @@ -24,7 +24,7 @@ import { } from '../../../typechain'; import {DAO__factory as DAO_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/core/dao/DAO.sol'; -import {upgradeManagingContract} from '../../test-utils/uups-upgradeable'; +import {upgradeCheckManagingContract} from '../../test-utils/uups-upgradeable'; import {findEvent, DAO_EVENTS} from '../../../utils/event'; import {flipBit} from '../../test-utils/bitmap'; @@ -332,7 +332,7 @@ describe('DAO', function () { it('from v1.0.0', async () => { legacyContractFactory = new DAO_V1_0_0__factory(signers[0]); - await upgradeManagingContract( + await upgradeCheckManagingContract( signers[0], signers[1], { diff --git a/packages/contracts/test/framework/dao/dao-registry.ts b/packages/contracts/test/framework/dao/dao-registry.ts index 5aa12fbca..1e77bd466 100644 --- a/packages/contracts/test/framework/dao/dao-registry.ts +++ b/packages/contracts/test/framework/dao/dao-registry.ts @@ -16,7 +16,7 @@ import {deployENSSubdomainRegistrar} from '../../test-utils/ens'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {deployWithProxy} from '../../test-utils/proxy'; import {UPGRADE_PERMISSIONS} from '../../test-utils/permissions'; -import {upgradeManagedContract} from '../../test-utils/uups-upgradeable'; +import {upgradeCheckManagedContract} from '../../test-utils/uups-upgradeable'; const EVENTS = { DAORegistered: 'DAORegistered', @@ -256,7 +256,7 @@ describe('DAORegistry', function () { it('from v1.0.0', async () => { legacyContractFactory = new DAORegistry_V1_0_0__factory(signers[0]); - await upgradeManagedContract( + await upgradeCheckManagedContract( signers[0], signers[1], managingDao, diff --git a/packages/contracts/test/framework/plugin/plugin-repo-registry.ts b/packages/contracts/test/framework/plugin/plugin-repo-registry.ts index 2d6cc34e1..a49c0dfde 100644 --- a/packages/contracts/test/framework/plugin/plugin-repo-registry.ts +++ b/packages/contracts/test/framework/plugin/plugin-repo-registry.ts @@ -18,7 +18,7 @@ import {deployENSSubdomainRegistrar} from '../../test-utils/ens'; import {ensDomainHash} from '../../../utils/ens'; import {deployWithProxy} from '../../test-utils/proxy'; import {UPGRADE_PERMISSIONS} from '../../test-utils/permissions'; -import {upgradeManagedContract} from '../../test-utils/uups-upgradeable'; +import {upgradeCheckManagedContract} from '../../test-utils/uups-upgradeable'; const EVENTS = { PluginRepoRegistered: 'PluginRepoRegistered', @@ -267,7 +267,7 @@ describe('PluginRepoRegistry', function () { signers[0] ); - await upgradeManagedContract( + await upgradeCheckManagedContract( signers[0], signers[1], managingDAO, diff --git a/packages/contracts/test/framework/plugin/plugin-repo.ts b/packages/contracts/test/framework/plugin/plugin-repo.ts index ee43d9dbf..905a966d9 100644 --- a/packages/contracts/test/framework/plugin/plugin-repo.ts +++ b/packages/contracts/test/framework/plugin/plugin-repo.ts @@ -19,7 +19,7 @@ import { } from '../../../typechain'; import {PluginRepo__factory as PluginRepo_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepo.sol'; -import {upgradeManagingContract} from '../../test-utils/uups-upgradeable'; +import {upgradeCheckManagingContract} from '../../test-utils/uups-upgradeable'; import { deployMockPluginSetup, @@ -85,7 +85,7 @@ describe('PluginRepo', function () { it('from v1.0.0', async () => { legacyContractFactory = new PluginRepo_V1_0_0__factory(signers[0]); - await upgradeManagingContract( + await upgradeCheckManagingContract( signers[0], signers[1], { diff --git a/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts b/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts index c1460770c..0bfd04178 100644 --- a/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts +++ b/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts @@ -20,7 +20,7 @@ import {ensDomainHash, ensLabelHash} from '../../../../utils/ens'; import {OZ_ERRORS} from '../../../test-utils/error'; import {setupResolver} from '../../../test-utils/ens'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; -import {upgradeManagedContract} from '../../../test-utils/uups-upgradeable'; +import {upgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; const REGISTER_ENS_SUBDOMAIN_PERMISSION_ID = ethers.utils.id( 'REGISTER_ENS_SUBDOMAIN_PERMISSION' @@ -298,7 +298,7 @@ describe('ENSSubdomainRegistrar', function () { signers[0] ); - await upgradeManagedContract( + await upgradeCheckManagedContract( signers[0], signers[1], managingDao, diff --git a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts index 53f3c7e36..aaefa21eb 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts @@ -48,7 +48,7 @@ import {OZ_ERRORS} from '../../../../test-utils/error'; import {UPGRADE_PERMISSIONS} from '../../../../test-utils/permissions'; import {deployWithProxy} from '../../../../test-utils/proxy'; import {getInterfaceID} from '../../../../test-utils/interfaces'; -import {upgradeManagedContract} from '../../../../test-utils/uups-upgradeable'; +import {upgradeCheckManagedContract} from '../../../../test-utils/uups-upgradeable'; export const addresslistVotingInterface = new ethers.utils.Interface([ 'function initialize(address,tuple(uint8,uint32,uint32,uint64,uint256),address[])', @@ -135,7 +135,7 @@ describe('AddresslistVoting', function () { it('from v1.0.0', async () => { legacyContractFactory = new AddresslistVoting_V1_0_0__factory(signers[0]); - await upgradeManagedContract( + await upgradeCheckManagedContract( signers[0], signers[1], dao, diff --git a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts index 4ec670422..6875b01bc 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts @@ -49,7 +49,7 @@ import {OZ_ERRORS} from '../../../../test-utils/error'; import {deployWithProxy} from '../../../../test-utils/proxy'; import {getInterfaceID} from '../../../../test-utils/interfaces'; import {UPGRADE_PERMISSIONS} from '../../../../test-utils/permissions'; -import {upgradeManagedContract} from '../../../../test-utils/uups-upgradeable'; +import {upgradeCheckManagedContract} from '../../../../test-utils/uups-upgradeable'; import {majorityVotingBaseInterface} from '../majority-voting'; export const tokenVotingInterface = new ethers.utils.Interface([ @@ -203,7 +203,7 @@ describe('TokenVoting', function () { it('from v1.0.0', async () => { legacyContractFactory = new TokenVoting_V1_0_0__factory(signers[0]); - await upgradeManagedContract( + await upgradeCheckManagedContract( signers[0], signers[1], dao, diff --git a/packages/contracts/test/plugins/governance/multisig/multisig.ts b/packages/contracts/test/plugins/governance/multisig/multisig.ts index 4bf2e637c..bb4acfeb4 100644 --- a/packages/contracts/test/plugins/governance/multisig/multisig.ts +++ b/packages/contracts/test/plugins/governance/multisig/multisig.ts @@ -42,7 +42,7 @@ import { import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; import {deployWithProxy} from '../../../test-utils/proxy'; import {getInterfaceID} from '../../../test-utils/interfaces'; -import {upgradeManagedContract} from '../../../test-utils/uups-upgradeable'; +import {upgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; export const multisigInterface = new ethers.utils.Interface([ 'function initialize(address,address[],tuple(bool,uint16))', @@ -202,7 +202,7 @@ describe('Multisig', function () { it('from v1.0.0', async () => { legacyContractFactory = new Multisig_V1_0_0__factory(signers[0]); - await upgradeManagedContract( + await upgradeCheckManagedContract( signers[0], signers[1], dao, diff --git a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts index 47dd233db..b9dcfab75 100644 --- a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts +++ b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts @@ -22,7 +22,7 @@ import BalanceTree from './src/balance-tree'; import {deployNewDAO} from '../../../test-utils/dao'; import {getInterfaceID} from '../../../test-utils/interfaces'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; -import {upgradeManagedContract} from '../../../test-utils/uups-upgradeable'; +import {upgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; const ZERO_BYTES32 = `0x${`0`.repeat(64)}`; @@ -86,7 +86,7 @@ describe('MerkleDistributor', function () { it('from v1.0.0', async () => { legacyContractFactory = new MerkleDistributor_V1_0_0__factory(signers[0]); - await upgradeManagedContract( + await upgradeCheckManagedContract( signers[0], signers[1], dao, diff --git a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts index 1e04ef55a..f17f32ad6 100644 --- a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts +++ b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts @@ -24,7 +24,7 @@ import {deployNewDAO} from '../../../test-utils/dao'; import {deployWithProxy} from '../../../test-utils/proxy'; import {getInterfaceID} from '../../../test-utils/interfaces'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; -import {upgradeManagedContract} from '../../../test-utils/uups-upgradeable'; +import {upgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; const MERKLE_MINT_PERMISSION_ID = ethers.utils.id('MERKLE_MINT_PERMISSION'); const MINT_PERMISSION_ID = ethers.utils.id('MINT_PERMISSION'); @@ -96,7 +96,7 @@ describe('MerkleMinter', function () { it('from v1.0.0', async () => { legacyContractFactory = new MerkleMinter_V1_0_0__factory(signers[0]); - await upgradeManagedContract( + await upgradeCheckManagedContract( signers[0], signers[1], managingDao, diff --git a/packages/contracts/test/test-utils/uups-upgradeable.ts b/packages/contracts/test/test-utils/uups-upgradeable.ts index 0056620ea..f72aa9db0 100644 --- a/packages/contracts/test/test-utils/uups-upgradeable.ts +++ b/packages/contracts/test/test-utils/uups-upgradeable.ts @@ -8,7 +8,8 @@ import {DAO, PluginRepo} from '../../typechain'; export const IMPLEMENTATION_SLOT = '0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc'; // bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1) -export async function upgradeManagedContract( +// Deploys and upgrades a contract that is managed by a DAO +export async function upgradeCheckManagedContract( deployer: SignerWithAddress, upgrader: SignerWithAddress, managingDao: DAO, @@ -30,6 +31,7 @@ export async function upgradeManagedContract( } ); + // Check that upgrade permission is required await expect( upgrades.upgradeProxy(proxy.address, to.connect(upgrader), { unsafeAllow: ['constructor'], @@ -56,7 +58,8 @@ export async function upgradeManagedContract( }); } -export async function upgradeManagingContract( +// Deploys and upgrades a contract that has its own permission manager +export async function upgradeCheckManagingContract( deployer: SignerWithAddress, upgrader: SignerWithAddress, initArgs: any, @@ -77,6 +80,7 @@ export async function upgradeManagingContract( } ); + // Check that upgrade permission is required await expect( upgrades.upgradeProxy(proxy.address, to.connect(upgrader), { unsafeAllow: ['constructor'], From 5ec1a61c2df16427fe3c716138d80030eeb85c5d Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Thu, 20 Jul 2023 13:49:04 +0200 Subject: [PATCH 16/31] fix: ambiguous artifacts --- .../plugin/plugin-setup-processor.ts | 3 ++- .../test/framework/utils/token-factory.ts | 5 +++-- .../test/test-utils/uups-upgradeable.ts | 2 +- .../test/token/erc20/governance-erc20.ts | 20 +++++++++---------- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/contracts/test/framework/plugin/plugin-setup-processor.ts b/packages/contracts/test/framework/plugin/plugin-setup-processor.ts index 5a4f9e220..b6f53f7cc 100644 --- a/packages/contracts/test/framework/plugin/plugin-setup-processor.ts +++ b/packages/contracts/test/framework/plugin/plugin-setup-processor.ts @@ -2,6 +2,7 @@ import {expect} from 'chai'; import {ethers} from 'hardhat'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {anyValue} from '@nomicfoundation/hardhat-chai-matchers/withArgs'; +import pluginUUPSUpgradeableArtifact from '../../../artifacts/src/core/plugin/PluginUUPSUpgradeable.sol/PluginUUPSUpgradeable.json'; import { PluginSetupProcessor, @@ -1975,7 +1976,7 @@ describe('Plugin Setup Processor', function () { beforeEach(async () => { // create a fake on the same plugin(proxy) address. - fake = await smock.fake('PluginUUPSUpgradeable', { + fake = await smock.fake(pluginUUPSUpgradeableArtifact, { address: proxy, }); diff --git a/packages/contracts/test/framework/utils/token-factory.ts b/packages/contracts/test/framework/utils/token-factory.ts index b0929bb21..79e4e36e2 100644 --- a/packages/contracts/test/framework/utils/token-factory.ts +++ b/packages/contracts/test/framework/utils/token-factory.ts @@ -1,6 +1,7 @@ import chai, {expect} from 'chai'; import {ethers} from 'hardhat'; import {FakeContract, MockContract, smock} from '@defi-wonderland/smock'; +import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import { ActionExecute__factory, @@ -20,7 +21,7 @@ import { TokenCreatedEvent, WrappedTokenEvent, } from '../../../typechain/TokenFactory'; -import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; +import merkleMinterArtifact from '../../../artifacts/src/plugins/token/MerkleMinter.sol/MerkleMinter.json'; chai.use(smock.matchers); @@ -73,7 +74,7 @@ describe('Core: TokenFactory', () => { ); const MerkleMinterBaseFactory = await smock.mock( - 'MerkleMinter' + merkleMinterArtifact ); merkleMinterBase = await MerkleMinterBaseFactory.deploy(); diff --git a/packages/contracts/test/test-utils/uups-upgradeable.ts b/packages/contracts/test/test-utils/uups-upgradeable.ts index f72aa9db0..b8b3939b6 100644 --- a/packages/contracts/test/test-utils/uups-upgradeable.ts +++ b/packages/contracts/test/test-utils/uups-upgradeable.ts @@ -2,7 +2,7 @@ import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; import {ContractFactory} from 'ethers'; import {upgrades} from 'hardhat'; -import {DAO, PluginRepo} from '../../typechain'; +import {DAO} from '../../typechain'; // See https://eips.ethereum.org/EIPS/eip-1967 export const IMPLEMENTATION_SLOT = diff --git a/packages/contracts/test/token/erc20/governance-erc20.ts b/packages/contracts/test/token/erc20/governance-erc20.ts index 20ea51d7d..7b6de4406 100644 --- a/packages/contracts/test/token/erc20/governance-erc20.ts +++ b/packages/contracts/test/token/erc20/governance-erc20.ts @@ -7,6 +7,10 @@ import { GovernanceERC20, GovernanceERC20__factory, IERC165Upgradeable__factory, + IERC20MintableUpgradeable__factory, + IERC20PermitUpgradeable__factory, + IERC20Upgradeable__factory, + IVotesUpgradeable__factory, } from '../../../typechain'; import {deployNewDAO} from '../../test-utils/dao'; import {OZ_ERRORS} from '../../test-utils/error'; @@ -107,17 +111,13 @@ describe('GovernanceERC20', function () { it('it supports all inherited interfaces', async () => { await Promise.all( [ - 'IERC20Upgradeable', - 'IERC20PermitUpgradeable', - 'IVotesUpgradeable', - 'IERC20MintableUpgradeable', + IERC20Upgradeable__factory.createInterface(), + IERC20PermitUpgradeable__factory.createInterface(), + IVotesUpgradeable__factory.createInterface(), + IERC20MintableUpgradeable__factory.createInterface(), ].map(async interfaceName => { - const iface = new ethers.utils.Interface( - // @ts-ignore - (await hre.artifacts.readArtifact(interfaceName)).abi - ); - expect(await token.supportsInterface(getInterfaceID(iface))).to.be - .true; + expect(await token.supportsInterface(getInterfaceID(interfaceName))) + .to.be.true; }) ); // We must check `IERC20MetadataUpgradeable` separately as it inherits from `IERC20Upgradeable` and we cannot get the isolated ABI. From 2ab0e5ccdb113e25b3aabe2eeca3301faef3f26b Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Thu, 20 Jul 2023 13:51:20 +0200 Subject: [PATCH 17/31] fix: use existing interface --- packages/contracts/test/framework/dao/dao-factory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts/test/framework/dao/dao-factory.ts b/packages/contracts/test/framework/dao/dao-factory.ts index 1ec69c648..5b1e9e114 100644 --- a/packages/contracts/test/framework/dao/dao-factory.ts +++ b/packages/contracts/test/framework/dao/dao-factory.ts @@ -647,7 +647,7 @@ describe('DAOFactory: ', function () { { to: dao.address, value: 0, - data: DAO__factory.createInterface().encodeFunctionData('grant', [ + data: daoInterface.encodeFunctionData('grant', [ dao.address, psp.address, ethers.utils.id('ROOT_PERMISSION'), From a9e40fba4c9adcb680cf97e79c497fdb9300dbd6 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Thu, 20 Jul 2023 15:16:56 +0200 Subject: [PATCH 18/31] feature: improved typing --- packages/contracts/deploy/helpers.ts | 5 ++- .../30_install-multisig-on-managing-dao.ts | 19 ++++----- .../test/framework/dao/dao-factory.ts | 40 +++++++++++-------- .../plugin/plugin-setup-processor.ts | 23 ++++++----- .../test/plugins/governance/admin/admin.ts | 6 ++- .../addresslist/addresslist-voting.ts | 3 +- .../majority-voting/token/token-voting.ts | 3 +- .../plugins/governance/multisig/multisig.ts | 6 +-- packages/contracts/test/upgrade/dao.ts | 7 +++- packages/contracts/utils/event.ts | 11 +++-- 10 files changed, 71 insertions(+), 52 deletions(-) diff --git a/packages/contracts/deploy/helpers.ts b/packages/contracts/deploy/helpers.ts index b23bfb20e..f371f5950 100644 --- a/packages/contracts/deploy/helpers.ts +++ b/packages/contracts/deploy/helpers.ts @@ -1,6 +1,7 @@ import {promises as fs} from 'fs'; import {ethers} from 'hardhat'; import {Contract} from 'ethers'; +import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {HardhatRuntimeEnvironment} from 'hardhat/types'; import IPFS from 'ipfs-http-client'; @@ -14,7 +15,7 @@ import { PluginRepo__factory, } from '../typechain'; import {VersionCreatedEvent} from '../typechain/PluginRepo'; -import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; +import {PluginRepoRegisteredEvent} from '../typechain/PluginRepoRegistry'; // TODO: Add support for L2 such as Arbitrum. (https://discuss.ens.domains/t/register-using-layer-2/688) // Make sure you own the ENS set in the {{NETWORK}}_ENS_DOMAIN variable in .env @@ -223,7 +224,7 @@ export async function createPluginRepo( ); await tx.wait(); - const event = await findEventTopicLog( + const event = await findEventTopicLog( tx, PluginRepoRegistry__factory.createInterface(), 'PluginRepoRegistered' diff --git a/packages/contracts/deploy/new/40_finalize-managing-dao/30_install-multisig-on-managing-dao.ts b/packages/contracts/deploy/new/40_finalize-managing-dao/30_install-multisig-on-managing-dao.ts index 05140492b..a54a7e4cb 100644 --- a/packages/contracts/deploy/new/40_finalize-managing-dao/30_install-multisig-on-managing-dao.ts +++ b/packages/contracts/deploy/new/40_finalize-managing-dao/30_install-multisig-on-managing-dao.ts @@ -108,17 +108,14 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { address: installationPreparedEvent.plugin, args: [ await multisigSetup.implementation(), - await Multisig__factory.createInterface().encodeFunctionData( - 'initialize', - [ - managingDAOAddress, - approvers, - { - onlyListed: listedOnly, - minApprovals: minApprovals, - }, - ] - ), + Multisig__factory.createInterface().encodeFunctionData('initialize', [ + managingDAOAddress, + approvers, + { + onlyListed: listedOnly, + minApprovals: minApprovals, + }, + ]), ], }); diff --git a/packages/contracts/test/framework/dao/dao-factory.ts b/packages/contracts/test/framework/dao/dao-factory.ts index 5b1e9e114..eb3cb625a 100644 --- a/packages/contracts/test/framework/dao/dao-factory.ts +++ b/packages/contracts/test/framework/dao/dao-factory.ts @@ -28,6 +28,9 @@ import { IERC165__factory, PluginRepoRegistry__factory, } from '../../../typechain'; +import {DAORegisteredEvent} from '../../../typechain/DAORegistry'; +import {InstallationPreparedEvent} from '../../../typechain/PluginSetupProcessor'; +import {PluginRepoRegisteredEvent} from '../../../typechain/PluginRepoRegistry'; import {deployENSSubdomainRegistrar} from '../../test-utils/ens'; import {deployPluginSetupProcessor} from '../../test-utils/plugin-setup-processor'; @@ -110,17 +113,18 @@ async function extractInfoFromCreateDaoTx(tx: any): Promise<{ helpers: any; permissions: any; }> { - const daoRegisteredEvent = await findEventTopicLog( + const daoRegisteredEvent = await findEventTopicLog( tx, DAORegistry__factory.createInterface(), EVENTS.DAORegistered ); - const installationPreparedEvent = await findEventTopicLog( - tx, - PluginSetupProcessor__factory.createInterface(), - EVENTS.InstallationPrepared - ); + const installationPreparedEvent = + await findEventTopicLog( + tx, + PluginSetupProcessor__factory.createInterface(), + EVENTS.InstallationPrepared + ); return { dao: daoRegisteredEvent.args.dao, @@ -247,7 +251,7 @@ describe('DAOFactory: ', function () { '0x00', '0x00' ); - const event = await findEventTopicLog( + const event = await findEventTopicLog( tx, PluginRepoRegistry__factory.createInterface(), EVENTS.PluginRepoRegistered @@ -584,11 +588,12 @@ describe('DAOFactory: ', function () { '0x11', '0x11' ); - const pluginRepoRegisteredEvent = await findEventTopicLog( - tx, - PluginRepoRegistry__factory.createInterface(), - EVENTS.PluginRepoRegistered - ); + const pluginRepoRegisteredEvent = + await findEventTopicLog( + tx, + PluginRepoRegistry__factory.createInterface(), + EVENTS.PluginRepoRegistered + ); adminPluginRepoAddress = pluginRepoRegisteredEvent.args.pluginRepo; // create dao with admin plugin. @@ -611,11 +616,12 @@ describe('DAOFactory: ', function () { ); tx = await daoFactory.createDao(daoSettings, [adminPluginInstallation]); { - const installationPreparedEvent = await findEventTopicLog( - tx, - PluginSetupProcessor__factory.createInterface(), - EVENTS.InstallationPrepared - ); + const installationPreparedEvent = + await findEventTopicLog( + tx, + PluginSetupProcessor__factory.createInterface(), + EVENTS.InstallationPrepared + ); const adminFactory = new Admin__factory(signers[0]); adminPlugin = adminFactory.attach( diff --git a/packages/contracts/test/framework/plugin/plugin-setup-processor.ts b/packages/contracts/test/framework/plugin/plugin-setup-processor.ts index 5a4f9e220..b7af6c512 100644 --- a/packages/contracts/test/framework/plugin/plugin-setup-processor.ts +++ b/packages/contracts/test/framework/plugin/plugin-setup-processor.ts @@ -31,6 +31,7 @@ import { PluginCloneableSetupV2Mock__factory, PluginCloneableSetupV1MockBad__factory, } from '../../../typechain'; +import {PluginRepoRegisteredEvent} from '../../../typechain/PluginRepoRegistry'; import {deployENSSubdomainRegistrar} from '../../test-utils/ens'; import {deployNewDAO, ZERO_BYTES32} from '../../test-utils/dao'; @@ -245,11 +246,12 @@ describe('Plugin Setup Processor', function () { buildMetadata ); - const PluginRepoRegisteredEvent1 = await findEventTopicLog( - tx, - PluginRepoRegistry__factory.createInterface(), - EVENTS.PluginRepoRegistered - ); + const PluginRepoRegisteredEvent1 = + await findEventTopicLog( + tx, + PluginRepoRegistry__factory.createInterface(), + EVENTS.PluginRepoRegistered + ); const PluginRepo = new PluginRepo__factory(signers[0]); repoU = PluginRepo.attach(PluginRepoRegisteredEvent1.args.pluginRepo); @@ -268,11 +270,12 @@ describe('Plugin Setup Processor', function () { buildMetadata ); - const PluginRepoRegisteredEvent2 = await findEventTopicLog( - tx, - PluginRepoRegistry__factory.createInterface(), - EVENTS.PluginRepoRegistered - ); + const PluginRepoRegisteredEvent2 = + await findEventTopicLog( + tx, + PluginRepoRegistry__factory.createInterface(), + EVENTS.PluginRepoRegistered + ); repoC = PluginRepo.attach(PluginRepoRegisteredEvent2.args.pluginRepo); await repoC.createVersion(1, setupCV1Bad.address, EMPTY_DATA, EMPTY_DATA); await repoC.createVersion(1, setupCV2.address, EMPTY_DATA, EMPTY_DATA); diff --git a/packages/contracts/test/plugins/governance/admin/admin.ts b/packages/contracts/test/plugins/governance/admin/admin.ts index f531cc673..3ffc1a526 100644 --- a/packages/contracts/test/plugins/governance/admin/admin.ts +++ b/packages/contracts/test/plugins/governance/admin/admin.ts @@ -24,6 +24,7 @@ import { DAO__factory, } from '../../../../typechain'; import {ProposalCreatedEvent} from '../../../../typechain/Admin'; +import {ExecutedEvent} from '../../../../typechain/IDAO'; // Permissions const EXECUTE_PROPOSAL_PERMISSION_ID = ethers.utils.id( @@ -237,7 +238,8 @@ describe('Admin', function () { dummyActions, allowFailureMap ); - const event = await findEventTopicLog( + + const event = await findEventTopicLog( tx, DAO__factory.createInterface(), DAO_EVENTS.EXECUTED @@ -258,7 +260,7 @@ describe('Admin', function () { const tx = await plugin.executeProposal(dummyMetadata, dummyActions, 0); - const event = await findEventTopicLog( + const event = await findEventTopicLog( tx, DAO__factory.createInterface(), DAO_EVENTS.EXECUTED diff --git a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts index 6ff1c4f83..a5aacc786 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts @@ -46,6 +46,7 @@ import { ProposalCreatedEvent, ProposalExecutedEvent, } from '../../../../../typechain/AddresslistVoting'; +import {ExecutedEvent} from '../../../../../typechain/IDAO'; export const addresslistVotingInterface = new ethers.utils.Interface([ 'function initialize(address,tuple(uint8,uint32,uint32,uint64,uint256),address[])', @@ -1026,7 +1027,7 @@ describe('AddresslistVoting', function () { .connect(signers[6]) .vote(id, VoteOption.Abstain, true); { - const event = await findEventTopicLog( + const event = await findEventTopicLog( tx, DAO__factory.createInterface(), DAO_EVENTS.EXECUTED diff --git a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts index c048bb480..125ff7e44 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts @@ -49,6 +49,7 @@ import { ProposalCreatedEvent, ProposalExecutedEvent, } from '../../../../../typechain/TokenVoting'; +import {ExecutedEvent} from '../../../../../typechain/IDAO'; export const tokenVotingInterface = new ethers.utils.Interface([ 'function initialize(address,tuple(uint8,uint32,uint32,uint64,uint256),address)', @@ -1382,7 +1383,7 @@ describe('TokenVoting', function () { .connect(signers[6]) .vote(id, VoteOption.Yes, true); { - const event = await findEventTopicLog( + const event = await findEventTopicLog( tx, DAO__factory.createInterface(), DAO_EVENTS.EXECUTED diff --git a/packages/contracts/test/plugins/governance/multisig/multisig.ts b/packages/contracts/test/plugins/governance/multisig/multisig.ts index 127829571..c07036794 100644 --- a/packages/contracts/test/plugins/governance/multisig/multisig.ts +++ b/packages/contracts/test/plugins/governance/multisig/multisig.ts @@ -1170,7 +1170,7 @@ describe('Multisig', function () { // `tryExecution` is turned on but the vote is not decided yet let tx = await multisig.connect(signers[1]).approve(id, true); await expect( - findEventTopicLog( + findEventTopicLog( tx, DAO__factory.createInterface(), DAO_EVENTS.EXECUTED @@ -1182,7 +1182,7 @@ describe('Multisig', function () { // `tryExecution` is turned off and the vote is decided tx = await multisig.connect(signers[2]).approve(id, false); await expect( - findEventTopicLog( + findEventTopicLog( tx, DAO__factory.createInterface(), DAO_EVENTS.EXECUTED @@ -1192,7 +1192,7 @@ describe('Multisig', function () { // `tryEarlyExecution` is turned on and the vote is decided tx = await multisig.connect(signers[3]).approve(id, true); { - const event = await findEventTopicLog( + const event = await findEventTopicLog( tx, DAO__factory.createInterface(), DAO_EVENTS.EXECUTED diff --git a/packages/contracts/test/upgrade/dao.ts b/packages/contracts/test/upgrade/dao.ts index ec3ba4ec7..5a8ef942d 100644 --- a/packages/contracts/test/upgrade/dao.ts +++ b/packages/contracts/test/upgrade/dao.ts @@ -15,6 +15,7 @@ import {UPGRADE_PERMISSIONS} from '../test-utils/permissions'; import {findEventTopicLog} from '../../utils/event'; import {readImplementationValueFromSlot} from '../../utils/storage'; import {getInterfaceID} from '../test-utils/interfaces'; +import {UpgradedEvent} from '../../typechain/DAO'; let signers: SignerWithAddress[]; let DAO_old: DAO_V1_0_0__factory; @@ -89,7 +90,11 @@ describe('DAO Upgrade', function () { // Check the emitted implementation. const emittedImplementation = ( - await findEventTopicLog(upgradeTx, DAO_old.interface, 'Upgraded') + await findEventTopicLog( + upgradeTx, + DAO_old.interface, + 'Upgraded' + ) ).args.implementation; expect(emittedImplementation).to.equal(daoCurrentImplementaion.address); diff --git a/packages/contracts/utils/event.ts b/packages/contracts/utils/event.ts index 48f09635c..bf6dcdd8b 100644 --- a/packages/contracts/utils/event.ts +++ b/packages/contracts/utils/event.ts @@ -1,7 +1,10 @@ import {ContractTransaction} from 'ethers'; import {Interface, LogDescription} from 'ethers/lib/utils'; -export async function findEvent(tx: ContractTransaction, eventName: string) { +export async function findEvent( + tx: ContractTransaction, + eventName: string +): Promise { const receipt = await tx.wait(); const event = (receipt.events || []).find(event => event.event === eventName); @@ -12,18 +15,18 @@ export async function findEvent(tx: ContractTransaction, eventName: string) { return event as unknown as T; } -export async function findEventTopicLog( +export async function findEventTopicLog( tx: ContractTransaction, iface: Interface, eventName: string -): Promise { +): Promise { const receipt = await tx.wait(); const topic = iface.getEventTopic(eventName); const log = receipt.logs.find(x => x.topics[0] === topic); if (!log) { throw new Error(`No logs found for the topic of event "${eventName}".`); } - return iface.parseLog(log); + return iface.parseLog(log) as LogDescription & (T | LogDescription); } export async function filterEvents(tx: any, eventName: string) { From 922d74ea928cd276d189e8006bd62545e3317f43 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Thu, 20 Jul 2023 15:56:24 +0200 Subject: [PATCH 19/31] fix: provide explicit paths to smock --- .../test/framework/utils/token-factory.ts | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/contracts/test/framework/utils/token-factory.ts b/packages/contracts/test/framework/utils/token-factory.ts index 79e4e36e2..0781b388d 100644 --- a/packages/contracts/test/framework/utils/token-factory.ts +++ b/packages/contracts/test/framework/utils/token-factory.ts @@ -16,12 +16,22 @@ import { TokenFactory, TokenFactory__factory, } from '../../../typechain'; + import {findEvent} from '../../../utils/event'; import { TokenCreatedEvent, WrappedTokenEvent, } from '../../../typechain/TokenFactory'; -import merkleMinterArtifact from '../../../artifacts/src/plugins/token/MerkleMinter.sol/MerkleMinter.json'; + +const daoArtifactPath = 'src/core/dao/DAO.sol:DAO'; +const governanceErc20ArtifactPath = + 'src/token/ERC20/governance/GovernanceERC20.sol:GovernanceERC20'; +const governanceWrappedErc20ArtifactPath = + 'src/token/ERC20/governance/GovernanceWrappedERC20.sol:GovernanceWrappedERC20'; +const merkleMinterArtifactPath = + 'src/plugins/token/MerkleMinter.sol:MerkleMinter'; +const tokenFactoryArtifactPath = + 'src/framework/utils/TokenFactory.sol:TokenFactory'; chai.use(smock.matchers); @@ -54,7 +64,7 @@ describe('Core: TokenFactory', () => { beforeEach(async () => { const GovernanceBaseFactory = await smock.mock( - 'GovernanceERC20' + governanceErc20ArtifactPath ); governanceBase = await GovernanceBaseFactory.deploy( zeroAddr, @@ -65,7 +75,7 @@ describe('Core: TokenFactory', () => { const GovernanceWrappedBaseFactory = await smock.mock( - 'GovernanceWrappedERC20' + governanceWrappedErc20ArtifactPath ); governanceWrappedBase = await GovernanceWrappedBaseFactory.deploy( zeroAddr, @@ -74,12 +84,12 @@ describe('Core: TokenFactory', () => { ); const MerkleMinterBaseFactory = await smock.mock( - merkleMinterArtifact + merkleMinterArtifactPath ); merkleMinterBase = await MerkleMinterBaseFactory.deploy(); const TokenFactoryFactory = await smock.mock( - 'TokenFactory' + tokenFactoryArtifactPath ); tokenFactory = await TokenFactoryFactory.deploy(); @@ -94,7 +104,7 @@ describe('Core: TokenFactory', () => { let dao: FakeContract; beforeEach(async () => { - dao = await smock.fake('src/core/dao/DAO.sol:DAO'); + dao = await smock.fake(daoArtifactPath); dao.isGranted.returns(true); dao.hasPermission.returns(true); dao.grant.returns(); @@ -123,7 +133,7 @@ describe('Core: TokenFactory', () => { it('should fail if token addr contains balanceOf, but returns different type', async () => { const erc20Contract = await smock.fake( - 'GovernanceERC20' + governanceErc20ArtifactPath ); erc20Contract.balanceOf.returns(true); @@ -146,7 +156,7 @@ describe('Core: TokenFactory', () => { it('should create a GovernanceWrappedERC20 clone', async () => { const erc20Contract = await smock.fake( - 'GovernanceERC20' + governanceErc20ArtifactPath ); erc20Contract.balanceOf.returns(2); @@ -177,7 +187,7 @@ describe('Core: TokenFactory', () => { it('should return MerkleMinter with 0x0', async () => { const erc20Contract = await smock.fake( - 'GovernanceERC20' + governanceErc20ArtifactPath ); erc20Contract.balanceOf.returns(2); From 306579de100b535c3dee806707b6b7c697206875 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Thu, 20 Jul 2023 16:09:57 +0200 Subject: [PATCH 20/31] fix: use strict comparision --- packages/contracts/test/framework/dao/dao-factory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts/test/framework/dao/dao-factory.ts b/packages/contracts/test/framework/dao/dao-factory.ts index eb3cb625a..ca3de366c 100644 --- a/packages/contracts/test/framework/dao/dao-factory.ts +++ b/packages/contracts/test/framework/dao/dao-factory.ts @@ -549,7 +549,7 @@ describe('DAOFactory: ', function () { // @ts-ignore receipt.logs.forEach(log => { - if (log.topics[0] == topic) installationAppliedEventCount++; + if (log.topics[0] === topic) installationAppliedEventCount++; }); expect(installationAppliedEventCount).to.equal(2); From 4679e66b12b417a8ba8c5dcdf8c1d1a28b508df5 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Thu, 20 Jul 2023 16:27:59 +0200 Subject: [PATCH 21/31] fix: missing await --- packages/contracts/utils/storage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts/utils/storage.ts b/packages/contracts/utils/storage.ts index 9ef36ff2f..80a13dfee 100644 --- a/packages/contracts/utils/storage.ts +++ b/packages/contracts/utils/storage.ts @@ -16,7 +16,7 @@ export async function readImplementationValuesFromSlot( export async function readImplementationValueFromSlot( contractAddress: string ): Promise { - return ethers.provider + return await ethers.provider .getStorageAt(contractAddress, IMPLEMENTATION_SLOT) .then(encoded => defaultAbiCoder.decode(['address'], encoded)[0]); } From 8f6b669878e33c0d6667e9a0c358c6cf8a156920 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Fri, 21 Jul 2023 07:53:39 +0200 Subject: [PATCH 22/31] fix: remove redundant typecast --- packages/contracts/test/framework/dao/dao-factory.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/contracts/test/framework/dao/dao-factory.ts b/packages/contracts/test/framework/dao/dao-factory.ts index ca3de366c..7aabb91b5 100644 --- a/packages/contracts/test/framework/dao/dao-factory.ts +++ b/packages/contracts/test/framework/dao/dao-factory.ts @@ -205,9 +205,7 @@ describe('DAOFactory: ', function () { ); // Deploy DAO Factory - const DAOFactory = new DAOFactory__factory( - signers[0] - ) as DAOFactory__factory; + const DAOFactory = new DAOFactory__factory(signers[0]); daoFactory = await DAOFactory.deploy(daoRegistry.address, psp.address); // Grant the `REGISTER_DAO_PERMISSION` permission to the `daoFactory` From 703e0b4d7ad38e465664be0f38234373d78f2fea Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Fri, 21 Jul 2023 07:55:13 +0200 Subject: [PATCH 23/31] fix: removed redundant ts-ignore --- packages/contracts/test/framework/dao/dao-factory.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/contracts/test/framework/dao/dao-factory.ts b/packages/contracts/test/framework/dao/dao-factory.ts index 7aabb91b5..cb5a9cef0 100644 --- a/packages/contracts/test/framework/dao/dao-factory.ts +++ b/packages/contracts/test/framework/dao/dao-factory.ts @@ -544,8 +544,6 @@ describe('DAOFactory: ', function () { ); let installationAppliedEventCount = 0; - - // @ts-ignore receipt.logs.forEach(log => { if (log.topics[0] === topic) installationAppliedEventCount++; }); From ff211a1d688f1bcdafa5fcb6d545c0e9ed600928 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Fri, 21 Jul 2023 09:27:05 +0200 Subject: [PATCH 24/31] feature: maintain checklists --- UPDATE_CHECKLIST.md | 1 + 1 file changed, 1 insertion(+) diff --git a/UPDATE_CHECKLIST.md b/UPDATE_CHECKLIST.md index 411aecc7b..fc7aea4f4 100644 --- a/UPDATE_CHECKLIST.md +++ b/UPDATE_CHECKLIST.md @@ -6,6 +6,7 @@ This checklist is seen as a guide to update the existing deployment. - [ ] Make sure you are using Node v16 - [ ] Verify that all changes of this update are reflected in [contracts/CHANGELOG.md](packages/contracts/CHANGELOG.md) by comparing the diff with the previous release commit. +- [ ] Check that tests checking for storage corruption using OZ's `hardhat-upgrades` package exists for every upgradeable contract and that test the upgradew from all prior versions to the current version - [ ] Check that all contracts that undergo an upgrade and - [ ] do require reinitialzation are reinitialized correctly by an `upgradeToAndCall` call to a respective initialization method with an incremented `reinitializer(X)` number - [ ] do NOT require reinitialzation are upgraded via `upgradeTo` and keep the same `reinitializer(X)` number in the respective initialization methods From eafe78a97b7d8a0e8a97a0255c6830adb30e69dc Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Fri, 21 Jul 2023 09:33:42 +0200 Subject: [PATCH 25/31] revert: wrong await / async --- packages/contracts/utils/storage.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/contracts/utils/storage.ts b/packages/contracts/utils/storage.ts index 80a13dfee..d1f0af135 100644 --- a/packages/contracts/utils/storage.ts +++ b/packages/contracts/utils/storage.ts @@ -3,7 +3,7 @@ import {defaultAbiCoder} from 'ethers/lib/utils'; import {IMPLEMENTATION_SLOT} from '../test/test-utils/uups-upgradeable'; -export async function readImplementationValuesFromSlot( +export function readImplementationValuesFromSlot( contractAddresses: string[] ): Promise { return Promise.all( @@ -13,10 +13,10 @@ export async function readImplementationValuesFromSlot( ); } -export async function readImplementationValueFromSlot( +export function readImplementationValueFromSlot( contractAddress: string ): Promise { - return await ethers.provider + return ethers.provider .getStorageAt(contractAddress, IMPLEMENTATION_SLOT) .then(encoded => defaultAbiCoder.decode(['address'], encoded)[0]); } From c3181e902ff92fcd09f2966eb1fa282e4558b1f0 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Fri, 21 Jul 2023 10:00:39 +0200 Subject: [PATCH 26/31] feature: improved checklist --- UPDATE_CHECKLIST.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UPDATE_CHECKLIST.md b/UPDATE_CHECKLIST.md index fc7aea4f4..d014f4577 100644 --- a/UPDATE_CHECKLIST.md +++ b/UPDATE_CHECKLIST.md @@ -6,7 +6,7 @@ This checklist is seen as a guide to update the existing deployment. - [ ] Make sure you are using Node v16 - [ ] Verify that all changes of this update are reflected in [contracts/CHANGELOG.md](packages/contracts/CHANGELOG.md) by comparing the diff with the previous release commit. -- [ ] Check that tests checking for storage corruption using OZ's `hardhat-upgrades` package exists for every upgradeable contract and that test the upgradew from all prior versions to the current version +- [ ] Check that storage corruption tests using OZ's `hardhat-upgrades` package (e.g., by using the methods in [uups-upgradeable.ts](packages/contracts/test/test-utils/uups-upgradeable.ts)) exist for every upgradeable contract and test the upgrade from all prior versions to the current version. - [ ] Check that all contracts that undergo an upgrade and - [ ] do require reinitialzation are reinitialized correctly by an `upgradeToAndCall` call to a respective initialization method with an incremented `reinitializer(X)` number - [ ] do NOT require reinitialzation are upgraded via `upgradeTo` and keep the same `reinitializer(X)` number in the respective initialization methods From fc2a96f08ebd4edefd1baa9c24a992c8b42ef40d Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Fri, 21 Jul 2023 10:10:02 +0200 Subject: [PATCH 27/31] feature: renamed function --- packages/contracts/test/core/dao/dao.ts | 4 ++-- packages/contracts/test/framework/dao/dao-registry.ts | 2 +- .../contracts/test/framework/plugin/plugin-repo-registry.ts | 4 ++-- packages/contracts/test/framework/plugin/plugin-repo.ts | 4 ++-- .../test/framework/utils/ens/ens-subdomain-registry.ts | 4 ++-- .../majority-voting/addresslist/addresslist-voting.ts | 4 ++-- .../governance/majority-voting/token/token-voting.ts | 4 ++-- .../contracts/test/plugins/governance/multisig/multisig.ts | 4 ++-- .../test/plugins/token/distribution/merkle-distributor.ts | 4 ++-- .../test/plugins/token/distribution/merkle-minter.ts | 4 ++-- packages/contracts/test/test-utils/uups-upgradeable.ts | 6 +++--- 11 files changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts index 072d12592..016bd30e9 100644 --- a/packages/contracts/test/core/dao/dao.ts +++ b/packages/contracts/test/core/dao/dao.ts @@ -24,7 +24,7 @@ import { } from '../../../typechain'; import {DAO__factory as DAO_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/core/dao/DAO.sol'; -import {upgradeCheckManagingContract} from '../../test-utils/uups-upgradeable'; +import {ozUpgradeCheckManagingContract} from '../../test-utils/uups-upgradeable'; import {findEvent, DAO_EVENTS} from '../../../utils/event'; import {flipBit} from '../../test-utils/bitmap'; @@ -332,7 +332,7 @@ describe('DAO', function () { it('from v1.0.0', async () => { legacyContractFactory = new DAO_V1_0_0__factory(signers[0]); - await upgradeCheckManagingContract( + await ozUpgradeCheckManagingContract( signers[0], signers[1], { diff --git a/packages/contracts/test/framework/dao/dao-registry.ts b/packages/contracts/test/framework/dao/dao-registry.ts index 1e77bd466..1e16562ec 100644 --- a/packages/contracts/test/framework/dao/dao-registry.ts +++ b/packages/contracts/test/framework/dao/dao-registry.ts @@ -16,7 +16,7 @@ import {deployENSSubdomainRegistrar} from '../../test-utils/ens'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {deployWithProxy} from '../../test-utils/proxy'; import {UPGRADE_PERMISSIONS} from '../../test-utils/permissions'; -import {upgradeCheckManagedContract} from '../../test-utils/uups-upgradeable'; +import {ozUpgradeCheckManagedContract} from '../../test-utils/uups-upgradeable'; const EVENTS = { DAORegistered: 'DAORegistered', diff --git a/packages/contracts/test/framework/plugin/plugin-repo-registry.ts b/packages/contracts/test/framework/plugin/plugin-repo-registry.ts index a49c0dfde..7c079a86e 100644 --- a/packages/contracts/test/framework/plugin/plugin-repo-registry.ts +++ b/packages/contracts/test/framework/plugin/plugin-repo-registry.ts @@ -18,7 +18,7 @@ import {deployENSSubdomainRegistrar} from '../../test-utils/ens'; import {ensDomainHash} from '../../../utils/ens'; import {deployWithProxy} from '../../test-utils/proxy'; import {UPGRADE_PERMISSIONS} from '../../test-utils/permissions'; -import {upgradeCheckManagedContract} from '../../test-utils/uups-upgradeable'; +import {ozUpgradeCheckManagedContract} from '../../test-utils/uups-upgradeable'; const EVENTS = { PluginRepoRegistered: 'PluginRepoRegistered', @@ -267,7 +267,7 @@ describe('PluginRepoRegistry', function () { signers[0] ); - await upgradeCheckManagedContract( + await ozUpgradeCheckManagedContract( signers[0], signers[1], managingDAO, diff --git a/packages/contracts/test/framework/plugin/plugin-repo.ts b/packages/contracts/test/framework/plugin/plugin-repo.ts index 905a966d9..82354cd2f 100644 --- a/packages/contracts/test/framework/plugin/plugin-repo.ts +++ b/packages/contracts/test/framework/plugin/plugin-repo.ts @@ -19,7 +19,7 @@ import { } from '../../../typechain'; import {PluginRepo__factory as PluginRepo_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepo.sol'; -import {upgradeCheckManagingContract} from '../../test-utils/uups-upgradeable'; +import {ozUpgradeCheckManagingContract} from '../../test-utils/uups-upgradeable'; import { deployMockPluginSetup, @@ -85,7 +85,7 @@ describe('PluginRepo', function () { it('from v1.0.0', async () => { legacyContractFactory = new PluginRepo_V1_0_0__factory(signers[0]); - await upgradeCheckManagingContract( + await ozUpgradeCheckManagingContract( signers[0], signers[1], { diff --git a/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts b/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts index 0bfd04178..5d6181558 100644 --- a/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts +++ b/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts @@ -20,7 +20,7 @@ import {ensDomainHash, ensLabelHash} from '../../../../utils/ens'; import {OZ_ERRORS} from '../../../test-utils/error'; import {setupResolver} from '../../../test-utils/ens'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; -import {upgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; +import {ozUpgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; const REGISTER_ENS_SUBDOMAIN_PERMISSION_ID = ethers.utils.id( 'REGISTER_ENS_SUBDOMAIN_PERMISSION' @@ -298,7 +298,7 @@ describe('ENSSubdomainRegistrar', function () { signers[0] ); - await upgradeCheckManagedContract( + await ozUpgradeCheckManagedContract( signers[0], signers[1], managingDao, diff --git a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts index 866a4c670..0168e2cbd 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts @@ -48,7 +48,7 @@ import {OZ_ERRORS} from '../../../../test-utils/error'; import {UPGRADE_PERMISSIONS} from '../../../../test-utils/permissions'; import {deployWithProxy} from '../../../../test-utils/proxy'; import {getInterfaceID} from '../../../../test-utils/interfaces'; -import {upgradeCheckManagedContract} from '../../../../test-utils/uups-upgradeable'; +import {ozUpgradeCheckManagedContract} from '../../../../test-utils/uups-upgradeable'; export const addresslistVotingInterface = new ethers.utils.Interface([ 'function initialize(address,tuple(uint8,uint32,uint32,uint64,uint256),address[])', @@ -135,7 +135,7 @@ describe('AddresslistVoting', function () { it('from v1.0.0', async () => { legacyContractFactory = new AddresslistVoting_V1_0_0__factory(signers[0]); - await upgradeCheckManagedContract( + await ozUpgradeCheckManagedContract( signers[0], signers[1], dao, diff --git a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts index 5133f6511..256f61085 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts @@ -49,7 +49,7 @@ import {OZ_ERRORS} from '../../../../test-utils/error'; import {deployWithProxy} from '../../../../test-utils/proxy'; import {getInterfaceID} from '../../../../test-utils/interfaces'; import {UPGRADE_PERMISSIONS} from '../../../../test-utils/permissions'; -import {upgradeCheckManagedContract} from '../../../../test-utils/uups-upgradeable'; +import {ozUpgradeCheckManagedContract} from '../../../../test-utils/uups-upgradeable'; import {majorityVotingBaseInterface} from '../majority-voting'; export const tokenVotingInterface = new ethers.utils.Interface([ @@ -203,7 +203,7 @@ describe('TokenVoting', function () { it('from v1.0.0', async () => { legacyContractFactory = new TokenVoting_V1_0_0__factory(signers[0]); - await upgradeCheckManagedContract( + await ozUpgradeCheckManagedContract( signers[0], signers[1], dao, diff --git a/packages/contracts/test/plugins/governance/multisig/multisig.ts b/packages/contracts/test/plugins/governance/multisig/multisig.ts index dbdd67fe6..5d923cdce 100644 --- a/packages/contracts/test/plugins/governance/multisig/multisig.ts +++ b/packages/contracts/test/plugins/governance/multisig/multisig.ts @@ -42,7 +42,7 @@ import { import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; import {deployWithProxy} from '../../../test-utils/proxy'; import {getInterfaceID} from '../../../test-utils/interfaces'; -import {upgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; +import {ozUpgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; export const multisigInterface = new ethers.utils.Interface([ 'function initialize(address,address[],tuple(bool,uint16))', @@ -202,7 +202,7 @@ describe('Multisig', function () { it('from v1.0.0', async () => { legacyContractFactory = new Multisig_V1_0_0__factory(signers[0]); - await upgradeCheckManagedContract( + await ozUpgradeCheckManagedContract( signers[0], signers[1], dao, diff --git a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts index b9dcfab75..ca940cd18 100644 --- a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts +++ b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts @@ -22,7 +22,7 @@ import BalanceTree from './src/balance-tree'; import {deployNewDAO} from '../../../test-utils/dao'; import {getInterfaceID} from '../../../test-utils/interfaces'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; -import {upgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; +import {ozUpgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; const ZERO_BYTES32 = `0x${`0`.repeat(64)}`; @@ -86,7 +86,7 @@ describe('MerkleDistributor', function () { it('from v1.0.0', async () => { legacyContractFactory = new MerkleDistributor_V1_0_0__factory(signers[0]); - await upgradeCheckManagedContract( + await ozUpgradeCheckManagedContract( signers[0], signers[1], dao, diff --git a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts index f17f32ad6..b39aa536c 100644 --- a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts +++ b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts @@ -24,7 +24,7 @@ import {deployNewDAO} from '../../../test-utils/dao'; import {deployWithProxy} from '../../../test-utils/proxy'; import {getInterfaceID} from '../../../test-utils/interfaces'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; -import {upgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; +import {ozUpgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; const MERKLE_MINT_PERMISSION_ID = ethers.utils.id('MERKLE_MINT_PERMISSION'); const MINT_PERMISSION_ID = ethers.utils.id('MINT_PERMISSION'); @@ -96,7 +96,7 @@ describe('MerkleMinter', function () { it('from v1.0.0', async () => { legacyContractFactory = new MerkleMinter_V1_0_0__factory(signers[0]); - await upgradeCheckManagedContract( + await ozUpgradeCheckManagedContract( signers[0], signers[1], managingDao, diff --git a/packages/contracts/test/test-utils/uups-upgradeable.ts b/packages/contracts/test/test-utils/uups-upgradeable.ts index b8b3939b6..2313f7f94 100644 --- a/packages/contracts/test/test-utils/uups-upgradeable.ts +++ b/packages/contracts/test/test-utils/uups-upgradeable.ts @@ -1,6 +1,6 @@ import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; -import {ContractFactory} from 'ethers'; +import {Contract, ContractFactory} from 'ethers'; import {upgrades} from 'hardhat'; import {DAO} from '../../typechain'; @@ -9,7 +9,7 @@ export const IMPLEMENTATION_SLOT = '0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc'; // bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1) // Deploys and upgrades a contract that is managed by a DAO -export async function upgradeCheckManagedContract( +export async function ozUpgradeCheckManagedContract( deployer: SignerWithAddress, upgrader: SignerWithAddress, managingDao: DAO, @@ -59,7 +59,7 @@ export async function upgradeCheckManagedContract( } // Deploys and upgrades a contract that has its own permission manager -export async function upgradeCheckManagingContract( +export async function ozUpgradeCheckManagingContract( deployer: SignerWithAddress, upgrader: SignerWithAddress, initArgs: any, From 3861c649cfd4008a024e73de7c9b85998d5c27df Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Fri, 21 Jul 2023 15:01:52 +0200 Subject: [PATCH 28/31] refactor: improved tests --- packages/contracts/test/core/dao/dao.ts | 44 +++++--- .../test/framework/dao/dao-registry.ts | 44 +++++--- .../framework/plugin/plugin-repo-registry.ts | 44 +++++--- .../test/framework/plugin/plugin-repo.ts | 38 +++++-- .../utils/ens/ens-subdomain-registry.ts | 45 +++++--- .../addresslist/addresslist-voting.ts | 45 +++++--- .../majority-voting/token/token-voting.ts | 45 +++++--- .../plugins/governance/multisig/multisig.ts | 49 ++++++--- .../token/distribution/merkle-distributor.ts | 45 +++++--- .../token/distribution/merkle-minter.ts | 45 +++++--- .../test/test-utils/uups-upgradeable.ts | 101 ++++++++++++------ packages/contracts/utils/storage.ts | 4 +- 12 files changed, 383 insertions(+), 166 deletions(-) diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts index 016bd30e9..03009b115 100644 --- a/packages/contracts/test/core/dao/dao.ts +++ b/packages/contracts/test/core/dao/dao.ts @@ -24,7 +24,10 @@ import { } from '../../../typechain'; import {DAO__factory as DAO_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/core/dao/DAO.sol'; -import {ozUpgradeCheckManagingContract} from '../../test-utils/uups-upgradeable'; +import { + getProtocolVersion, + ozUpgradeCheckManagingContract, +} from '../../test-utils/uups-upgradeable'; import {findEvent, DAO_EVENTS} from '../../../utils/event'; import {flipBit} from '../../test-utils/bitmap'; @@ -332,20 +335,33 @@ describe('DAO', function () { it('from v1.0.0', async () => { legacyContractFactory = new DAO_V1_0_0__factory(signers[0]); - await ozUpgradeCheckManagingContract( - signers[0], - signers[1], - { - metadata: dummyMetadata1, - initialOwner: signers[0].address, - trustedForwarder: dummyAddress1, - daoURI: daoExampleURI, - }, - 'initialize', - legacyContractFactory, - currentContractFactory, - UPGRADE_PERMISSIONS.UPGRADE_DAO_PERMISSION_ID + const {fromImplementation, toImplementation} = + await ozUpgradeCheckManagingContract( + signers[0], + signers[1], + { + metadata: dummyMetadata1, + initialOwner: signers[0].address, + trustedForwarder: dummyAddress1, + daoURI: daoExampleURI, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_DAO_PERMISSION_ID + ); + 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(CURRENT_PROTOCOL_VERSION); }); }); diff --git a/packages/contracts/test/framework/dao/dao-registry.ts b/packages/contracts/test/framework/dao/dao-registry.ts index 1e16562ec..a75e135b0 100644 --- a/packages/contracts/test/framework/dao/dao-registry.ts +++ b/packages/contracts/test/framework/dao/dao-registry.ts @@ -16,7 +16,11 @@ import {deployENSSubdomainRegistrar} from '../../test-utils/ens'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {deployWithProxy} from '../../test-utils/proxy'; import {UPGRADE_PERMISSIONS} from '../../test-utils/permissions'; -import {ozUpgradeCheckManagedContract} from '../../test-utils/uups-upgradeable'; +import { + getProtocolVersion, + ozUpgradeCheckManagedContract, +} from '../../test-utils/uups-upgradeable'; +import {CURRENT_PROTOCOL_VERSION} from '../../test-utils/protocol-version'; const EVENTS = { DAORegistered: 'DAORegistered', @@ -256,19 +260,33 @@ describe('DAORegistry', function () { it('from v1.0.0', async () => { legacyContractFactory = new DAORegistry_V1_0_0__factory(signers[0]); - await upgradeCheckManagedContract( - signers[0], - signers[1], - managingDao, - { - dao: managingDao.address, - ensSubdomainRegistrar: ensSubdomainRegistrar.address, - }, - 'initialize', - legacyContractFactory, - currentContractFactory, - UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID + const {fromImplementation, toImplementation} = + await ozUpgradeCheckManagedContract( + signers[0], + signers[1], + managingDao, + { + dao: managingDao.address, + ensSubdomainRegistrar: ensSubdomainRegistrar.address, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID + ); + + expect(toImplementation).to.equal(fromImplementation); // The implementation was not changed from 1.0.0 to the current version + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) ); + + expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); + expect(fromProtocolVersion).to.deep.equal([1, 0, 0]); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); }); }); diff --git a/packages/contracts/test/framework/plugin/plugin-repo-registry.ts b/packages/contracts/test/framework/plugin/plugin-repo-registry.ts index 7c079a86e..45b9e3fb2 100644 --- a/packages/contracts/test/framework/plugin/plugin-repo-registry.ts +++ b/packages/contracts/test/framework/plugin/plugin-repo-registry.ts @@ -18,7 +18,11 @@ import {deployENSSubdomainRegistrar} from '../../test-utils/ens'; import {ensDomainHash} from '../../../utils/ens'; import {deployWithProxy} from '../../test-utils/proxy'; import {UPGRADE_PERMISSIONS} from '../../test-utils/permissions'; -import {ozUpgradeCheckManagedContract} from '../../test-utils/uups-upgradeable'; +import { + getProtocolVersion, + ozUpgradeCheckManagedContract, +} from '../../test-utils/uups-upgradeable'; +import {CURRENT_PROTOCOL_VERSION} from '../../test-utils/protocol-version'; const EVENTS = { PluginRepoRegistered: 'PluginRepoRegistered', @@ -267,19 +271,33 @@ describe('PluginRepoRegistry', function () { signers[0] ); - await ozUpgradeCheckManagedContract( - signers[0], - signers[1], - managingDAO, - { - dao: managingDAO.address, - ensSubdomainRegistrar: ensSubdomainRegistrar.address, - }, - 'initialize', - legacyContractFactory, - currentContractFactory, - UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID + const {fromImplementation, toImplementation} = + await ozUpgradeCheckManagedContract( + signers[0], + signers[1], + managingDAO, + { + dao: managingDAO.address, + ensSubdomainRegistrar: ensSubdomainRegistrar.address, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID + ); + + expect(toImplementation).to.equal(fromImplementation); // The implementation was not changed from 1.0.0 to the current version + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) ); + + expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); + expect(fromProtocolVersion).to.deep.equal([1, 0, 0]); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); }); }); diff --git a/packages/contracts/test/framework/plugin/plugin-repo.ts b/packages/contracts/test/framework/plugin/plugin-repo.ts index 82354cd2f..46242b520 100644 --- a/packages/contracts/test/framework/plugin/plugin-repo.ts +++ b/packages/contracts/test/framework/plugin/plugin-repo.ts @@ -19,7 +19,10 @@ import { } from '../../../typechain'; import {PluginRepo__factory as PluginRepo_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepo.sol'; -import {ozUpgradeCheckManagingContract} from '../../test-utils/uups-upgradeable'; +import { + getProtocolVersion, + ozUpgradeCheckManagingContract, +} from '../../test-utils/uups-upgradeable'; import { deployMockPluginSetup, @@ -85,17 +88,30 @@ describe('PluginRepo', function () { it('from v1.0.0', async () => { legacyContractFactory = new PluginRepo_V1_0_0__factory(signers[0]); - await ozUpgradeCheckManagingContract( - signers[0], - signers[1], - { - initialOwner: signers[0].address, - }, - 'initialize', - legacyContractFactory, - currentContractFactory, - UPGRADE_PERMISSIONS.UPGRADE_REPO_PERMISSION_ID + const {fromImplementation, toImplementation} = + await ozUpgradeCheckManagingContract( + signers[0], + signers[1], + { + initialOwner: signers[0].address, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_REPO_PERMISSION_ID + ); + 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(CURRENT_PROTOCOL_VERSION); }); }); diff --git a/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts b/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts index 5d6181558..ee90ffe62 100644 --- a/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts +++ b/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts @@ -20,7 +20,11 @@ import {ensDomainHash, ensLabelHash} from '../../../../utils/ens'; import {OZ_ERRORS} from '../../../test-utils/error'; import {setupResolver} from '../../../test-utils/ens'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; -import {ozUpgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; +import { + getProtocolVersion, + ozUpgradeCheckManagedContract, +} from '../../../test-utils/uups-upgradeable'; +import {CURRENT_PROTOCOL_VERSION} from '../../../test-utils/protocol-version'; const REGISTER_ENS_SUBDOMAIN_PERMISSION_ID = ethers.utils.id( 'REGISTER_ENS_SUBDOMAIN_PERMISSION' @@ -298,20 +302,33 @@ describe('ENSSubdomainRegistrar', function () { signers[0] ); - await ozUpgradeCheckManagedContract( - signers[0], - signers[1], - managingDao, - { - managingDao: managingDao.address, - ens: ens.address, - parentDomain: ensDomainHash('test'), - }, - 'initialize', - legacyContractFactory, - currentContractFactory, - UPGRADE_PERMISSIONS.UPGRADE_REGISTRAR_PERMISSION_ID + const {fromImplementation, toImplementation} = + await ozUpgradeCheckManagedContract( + signers[0], + signers[1], + managingDao, + { + managingDao: managingDao.address, + ens: ens.address, + parentDomain: ensDomainHash('test'), + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_REGISTRAR_PERMISSION_ID + ); + expect(toImplementation).to.equal(fromImplementation); // The implementation was not changed from 1.0.0 to the current version + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) ); + + expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); + expect(fromProtocolVersion).to.deep.equal([1, 0, 0]); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); }); diff --git a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts index 0168e2cbd..29237b428 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts @@ -48,7 +48,11 @@ import {OZ_ERRORS} from '../../../../test-utils/error'; import {UPGRADE_PERMISSIONS} from '../../../../test-utils/permissions'; import {deployWithProxy} from '../../../../test-utils/proxy'; import {getInterfaceID} from '../../../../test-utils/interfaces'; -import {ozUpgradeCheckManagedContract} from '../../../../test-utils/uups-upgradeable'; +import { + getProtocolVersion, + ozUpgradeCheckManagedContract, +} from '../../../../test-utils/uups-upgradeable'; +import {CURRENT_PROTOCOL_VERSION} from '../../../../test-utils/protocol-version'; export const addresslistVotingInterface = new ethers.utils.Interface([ 'function initialize(address,tuple(uint8,uint32,uint32,uint64,uint256),address[])', @@ -135,20 +139,33 @@ describe('AddresslistVoting', function () { it('from v1.0.0', async () => { legacyContractFactory = new AddresslistVoting_V1_0_0__factory(signers[0]); - await ozUpgradeCheckManagedContract( - signers[0], - signers[1], - dao, - { - dao: dao.address, - votingSettings: votingSettings, - members: [signers[0].address, signers[1].address], - }, - 'initialize', - legacyContractFactory, - currentContractFactory, - UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + const {fromImplementation, toImplementation} = + await ozUpgradeCheckManagedContract( + signers[0], + signers[1], + dao, + { + dao: dao.address, + votingSettings: votingSettings, + members: [signers[0].address, signers[1].address], + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + ); + 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.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.0.0 to the current version + expect(fromProtocolVersion).to.deep.equal([1, 0, 0]); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); }); diff --git a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts index 256f61085..c40a0a72d 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts @@ -49,8 +49,12 @@ import {OZ_ERRORS} from '../../../../test-utils/error'; import {deployWithProxy} from '../../../../test-utils/proxy'; import {getInterfaceID} from '../../../../test-utils/interfaces'; import {UPGRADE_PERMISSIONS} from '../../../../test-utils/permissions'; -import {ozUpgradeCheckManagedContract} from '../../../../test-utils/uups-upgradeable'; +import { + getProtocolVersion, + ozUpgradeCheckManagedContract, +} from '../../../../test-utils/uups-upgradeable'; import {majorityVotingBaseInterface} from '../majority-voting'; +import {CURRENT_PROTOCOL_VERSION} from '../../../../test-utils/protocol-version'; export const tokenVotingInterface = new ethers.utils.Interface([ 'function initialize(address,tuple(uint8,uint32,uint32,uint64,uint256),address)', @@ -203,20 +207,33 @@ describe('TokenVoting', function () { it('from v1.0.0', async () => { legacyContractFactory = new TokenVoting_V1_0_0__factory(signers[0]); - await ozUpgradeCheckManagedContract( - signers[0], - signers[1], - dao, - { - dao: dao.address, - votingSettings: votingSettings, - token: governanceErc20Mock.address, - }, - 'initialize', - legacyContractFactory, - currentContractFactory, - UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + const {fromImplementation, toImplementation} = + await ozUpgradeCheckManagedContract( + signers[0], + signers[1], + dao, + { + dao: dao.address, + votingSettings: votingSettings, + token: governanceErc20Mock.address, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + ); + 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.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.0.0 to the current version + expect(fromProtocolVersion).to.deep.equal([1, 0, 0]); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); }); diff --git a/packages/contracts/test/plugins/governance/multisig/multisig.ts b/packages/contracts/test/plugins/governance/multisig/multisig.ts index 5d923cdce..a68d1236c 100644 --- a/packages/contracts/test/plugins/governance/multisig/multisig.ts +++ b/packages/contracts/test/plugins/governance/multisig/multisig.ts @@ -42,7 +42,11 @@ import { import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; import {deployWithProxy} from '../../../test-utils/proxy'; import {getInterfaceID} from '../../../test-utils/interfaces'; -import {ozUpgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; +import { + getProtocolVersion, + ozUpgradeCheckManagedContract, +} from '../../../test-utils/uups-upgradeable'; +import {CURRENT_PROTOCOL_VERSION} from '../../../test-utils/protocol-version'; export const multisigInterface = new ethers.utils.Interface([ 'function initialize(address,address[],tuple(bool,uint16))', @@ -202,20 +206,37 @@ describe('Multisig', function () { it('from v1.0.0', async () => { legacyContractFactory = new Multisig_V1_0_0__factory(signers[0]); - await ozUpgradeCheckManagedContract( - signers[0], - signers[1], - dao, - { - dao: dao.address, - members: [signers[0].address, signers[1].address, signers[2].address], - multisigSettings: multisigSettings, - }, - 'initialize', - legacyContractFactory, - currentContractFactory, - UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + const {fromImplementation, toImplementation} = + await ozUpgradeCheckManagedContract( + signers[0], + signers[1], + dao, + { + dao: dao.address, + members: [ + signers[0].address, + signers[1].address, + signers[2].address, + ], + multisigSettings: multisigSettings, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + ); + 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.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.0.0 to the current version + expect(fromProtocolVersion).to.deep.equal([1, 0, 0]); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); }); diff --git a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts index ca940cd18..6123926fa 100644 --- a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts +++ b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts @@ -22,7 +22,11 @@ import BalanceTree from './src/balance-tree'; import {deployNewDAO} from '../../../test-utils/dao'; import {getInterfaceID} from '../../../test-utils/interfaces'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; -import {ozUpgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; +import { + getProtocolVersion, + ozUpgradeCheckManagedContract, +} from '../../../test-utils/uups-upgradeable'; +import {CURRENT_PROTOCOL_VERSION} from '../../../test-utils/protocol-version'; const ZERO_BYTES32 = `0x${`0`.repeat(64)}`; @@ -86,20 +90,33 @@ describe('MerkleDistributor', function () { it('from v1.0.0', async () => { legacyContractFactory = new MerkleDistributor_V1_0_0__factory(signers[0]); - await ozUpgradeCheckManagedContract( - signers[0], - signers[1], - dao, - { - dao: dao.address, - token: token.address, - merkleRoot: ZERO_BYTES32, - }, - 'initialize', - legacyContractFactory, - currentContractFactory, - UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + const {fromImplementation, toImplementation} = + await ozUpgradeCheckManagedContract( + signers[0], + signers[1], + dao, + { + dao: dao.address, + token: token.address, + merkleRoot: ZERO_BYTES32, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + ); + expect(toImplementation).to.equal(fromImplementation); // The build did not change + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) ); + + expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.0.0 to the current version + expect(fromProtocolVersion).to.deep.equal([1, 0, 0]); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); }); diff --git a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts index b39aa536c..82d6091a9 100644 --- a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts +++ b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts @@ -24,7 +24,11 @@ import {deployNewDAO} from '../../../test-utils/dao'; import {deployWithProxy} from '../../../test-utils/proxy'; import {getInterfaceID} from '../../../test-utils/interfaces'; import {UPGRADE_PERMISSIONS} from '../../../test-utils/permissions'; -import {ozUpgradeCheckManagedContract} from '../../../test-utils/uups-upgradeable'; +import { + getProtocolVersion, + ozUpgradeCheckManagedContract, +} from '../../../test-utils/uups-upgradeable'; +import {CURRENT_PROTOCOL_VERSION} from '../../../test-utils/protocol-version'; const MERKLE_MINT_PERMISSION_ID = ethers.utils.id('MERKLE_MINT_PERMISSION'); const MINT_PERMISSION_ID = ethers.utils.id('MINT_PERMISSION'); @@ -96,20 +100,33 @@ describe('MerkleMinter', function () { it('from v1.0.0', async () => { legacyContractFactory = new MerkleMinter_V1_0_0__factory(signers[0]); - await ozUpgradeCheckManagedContract( - signers[0], - signers[1], - managingDao, - { - dao: managingDao.address, - token: token.address, - merkleDistributor: distributorBase.address, - }, - 'initialize', - legacyContractFactory, - currentContractFactory, - UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + const {fromImplementation, toImplementation} = + await ozUpgradeCheckManagedContract( + signers[0], + signers[1], + managingDao, + { + dao: managingDao.address, + token: token.address, + merkleDistributor: distributorBase.address, + }, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + ); + expect(toImplementation).to.equal(fromImplementation); // The build did not change + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) + ); + + expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.0.0 to the current version + expect(fromProtocolVersion).to.deep.equal([1, 0, 0]); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); }); diff --git a/packages/contracts/test/test-utils/uups-upgradeable.ts b/packages/contracts/test/test-utils/uups-upgradeable.ts index 2313f7f94..3b271ef77 100644 --- a/packages/contracts/test/test-utils/uups-upgradeable.ts +++ b/packages/contracts/test/test-utils/uups-upgradeable.ts @@ -1,12 +1,10 @@ import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; -import {Contract, ContractFactory} from 'ethers'; -import {upgrades} from 'hardhat'; -import {DAO} from '../../typechain'; - -// See https://eips.ethereum.org/EIPS/eip-1967 -export const IMPLEMENTATION_SLOT = - '0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc'; // bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1) +import {Contract, ContractFactory, errors} from 'ethers'; +import {ethers, upgrades} from 'hardhat'; +import {DAO, UUPSUpgradeable} from '../../typechain'; +import {readImplementationValueFromSlot} from '../../utils/storage'; +import {CURRENT_PROTOCOL_VERSION} from './protocol-version'; // Deploys and upgrades a contract that is managed by a DAO export async function ozUpgradeCheckManagedContract( @@ -18,17 +16,16 @@ export async function ozUpgradeCheckManagedContract( from: ContractFactory, to: ContractFactory, upgradePermissionId: string -) { +): Promise<{ + proxy: Contract; + fromImplementation: string; + toImplementation: string; +}> { // Deploy the proxy - const proxy = await upgrades.deployProxy( + const {proxy, implementation: fromImplementation} = await deployProxy( from.connect(deployer), - Object.values(initArgs), - { - kind: 'uups', - initializer: initializerName, - unsafeAllow: ['constructor'], - constructorArgs: [], - } + initArgs, + initializerName ); // Check that upgrade permission is required @@ -52,10 +49,9 @@ export async function ozUpgradeCheckManagedContract( .grant(proxy.address, upgrader.address, upgradePermissionId); // Upgrade the proxy - await upgrades.upgradeProxy(proxy.address, to.connect(upgrader), { - unsafeAllow: ['constructor'], - constructorArgs: [], - }); + const toImplementation = await upgradeProxy(proxy, to.connect(upgrader)); + + return {proxy, fromImplementation, toImplementation}; } // Deploys and upgrades a contract that has its own permission manager @@ -67,17 +63,16 @@ export async function ozUpgradeCheckManagingContract( from: ContractFactory, to: ContractFactory, upgradePermissionId: string -) { +): Promise<{ + proxy: Contract; + fromImplementation: string; + toImplementation: string; +}> { // Deploy the proxy - const proxy = await upgrades.deployProxy( + const {proxy, implementation: fromImplementation} = await deployProxy( from.connect(deployer), - Object.values(initArgs), - { - kind: 'uups', - initializer: initializerName, - unsafeAllow: ['constructor'], - constructorArgs: [], - } + initArgs, + initializerName ); // Check that upgrade permission is required @@ -96,8 +91,54 @@ export async function ozUpgradeCheckManagingContract( .grant(proxy.address, upgrader.address, upgradePermissionId); // Upgrade the proxy - await upgrades.upgradeProxy(proxy.address, to.connect(upgrader), { + const toImplementation = await upgradeProxy(proxy, to.connect(upgrader)); + + return {proxy, fromImplementation, toImplementation}; +} + +async function deployProxy( + factory: ContractFactory, + initArgs: any, + initializerName: string +): Promise<{proxy: Contract; implementation: string}> { + let proxy = await upgrades.deployProxy(factory, Object.values(initArgs), { + kind: 'uups', + initializer: initializerName, unsafeAllow: ['constructor'], constructorArgs: [], }); + + const implementation = await readImplementationValueFromSlot(proxy.address); + + return {proxy, implementation}; +} + +async function upgradeProxy( + proxy: Contract, + factory: ContractFactory +): Promise { + proxy = await upgrades.upgradeProxy(proxy.address, factory, { + unsafeAllow: ['constructor'], + constructorArgs: [], + }); + + const toImplementation = await readImplementationValueFromSlot(proxy.address); + return toImplementation; +} + +export async function getProtocolVersion( + contract: Contract +): Promise<[number, number, number]> { + let protocolVersion: [number, number, number]; + try { + contract.interface.getFunction('protocolVersion'); + protocolVersion = await contract.protocolVersion(); + } catch (error) { + if (error.code === errors.INVALID_ARGUMENT) { + protocolVersion = [1, 0, 0]; + } else { + throw error; + } + } + return protocolVersion; } diff --git a/packages/contracts/utils/storage.ts b/packages/contracts/utils/storage.ts index d1f0af135..cf2cd3087 100644 --- a/packages/contracts/utils/storage.ts +++ b/packages/contracts/utils/storage.ts @@ -1,7 +1,9 @@ import {ethers} from 'hardhat'; import {defaultAbiCoder} from 'ethers/lib/utils'; -import {IMPLEMENTATION_SLOT} from '../test/test-utils/uups-upgradeable'; +// See https://eips.ethereum.org/EIPS/eip-1967 +export const IMPLEMENTATION_SLOT = + '0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc'; // bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1) export function readImplementationValuesFromSlot( contractAddresses: string[] From ee567dae5664ced72e408d5a981c251ab6a88453 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Fri, 21 Jul 2023 16:31:23 +0200 Subject: [PATCH 29/31] fix: remove redundant imports --- packages/contracts/test/test-utils/uups-upgradeable.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/contracts/test/test-utils/uups-upgradeable.ts b/packages/contracts/test/test-utils/uups-upgradeable.ts index 3b271ef77..cc234c6bd 100644 --- a/packages/contracts/test/test-utils/uups-upgradeable.ts +++ b/packages/contracts/test/test-utils/uups-upgradeable.ts @@ -1,10 +1,9 @@ import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; import {Contract, ContractFactory, errors} from 'ethers'; -import {ethers, upgrades} from 'hardhat'; -import {DAO, UUPSUpgradeable} from '../../typechain'; +import {upgrades} from 'hardhat'; +import {DAO} from '../../typechain'; import {readImplementationValueFromSlot} from '../../utils/storage'; -import {CURRENT_PROTOCOL_VERSION} from './protocol-version'; // Deploys and upgrades a contract that is managed by a DAO export async function ozUpgradeCheckManagedContract( From b2cec9ef7a800d05e8a87568aebbe44dbf65767d Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Fri, 21 Jul 2023 16:47:24 +0200 Subject: [PATCH 30/31] refactor: remove method --- .../test/test-utils/uups-upgradeable.ts | 74 +++++++++---------- 1 file changed, 33 insertions(+), 41 deletions(-) diff --git a/packages/contracts/test/test-utils/uups-upgradeable.ts b/packages/contracts/test/test-utils/uups-upgradeable.ts index cc234c6bd..b7eed96c4 100644 --- a/packages/contracts/test/test-utils/uups-upgradeable.ts +++ b/packages/contracts/test/test-utils/uups-upgradeable.ts @@ -21,10 +21,19 @@ export async function ozUpgradeCheckManagedContract( toImplementation: string; }> { // Deploy the proxy - const {proxy, implementation: fromImplementation} = await deployProxy( + let proxy = await upgrades.deployProxy( from.connect(deployer), - initArgs, - initializerName + Object.values(initArgs), + { + kind: 'uups', + initializer: initializerName, + unsafeAllow: ['constructor'], + constructorArgs: [], + } + ); + + const fromImplementation = await readImplementationValueFromSlot( + proxy.address ); // Check that upgrade permission is required @@ -48,7 +57,12 @@ export async function ozUpgradeCheckManagedContract( .grant(proxy.address, upgrader.address, upgradePermissionId); // Upgrade the proxy - const toImplementation = await upgradeProxy(proxy, to.connect(upgrader)); + await upgrades.upgradeProxy(proxy.address, to.connect(upgrader), { + unsafeAllow: ['constructor'], + constructorArgs: [], + }); + + const toImplementation = await readImplementationValueFromSlot(proxy.address); return {proxy, fromImplementation, toImplementation}; } @@ -68,19 +82,22 @@ export async function ozUpgradeCheckManagingContract( toImplementation: string; }> { // Deploy the proxy - const {proxy, implementation: fromImplementation} = await deployProxy( + let proxy = await upgrades.deployProxy( from.connect(deployer), - initArgs, - initializerName + Object.values(initArgs), + { + kind: 'uups', + initializer: initializerName, + unsafeAllow: ['constructor'], + constructorArgs: [], + } ); + const fromImplementation = await readImplementationValueFromSlot( + proxy.address + ); // Check that upgrade permission is required - await expect( - upgrades.upgradeProxy(proxy.address, to.connect(upgrader), { - unsafeAllow: ['constructor'], - constructorArgs: [], - }) - ) + await expect(upgrades.upgradeProxy(proxy, to.connect(upgrader))) .to.be.revertedWithCustomError(proxy, 'Unauthorized') .withArgs(proxy.address, upgrader.address, upgradePermissionId); @@ -90,39 +107,14 @@ export async function ozUpgradeCheckManagingContract( .grant(proxy.address, upgrader.address, upgradePermissionId); // Upgrade the proxy - const toImplementation = await upgradeProxy(proxy, to.connect(upgrader)); - - return {proxy, fromImplementation, toImplementation}; -} - -async function deployProxy( - factory: ContractFactory, - initArgs: any, - initializerName: string -): Promise<{proxy: Contract; implementation: string}> { - let proxy = await upgrades.deployProxy(factory, Object.values(initArgs), { - kind: 'uups', - initializer: initializerName, - unsafeAllow: ['constructor'], - constructorArgs: [], - }); - - const implementation = await readImplementationValueFromSlot(proxy.address); - - return {proxy, implementation}; -} - -async function upgradeProxy( - proxy: Contract, - factory: ContractFactory -): Promise { - proxy = await upgrades.upgradeProxy(proxy.address, factory, { + await upgrades.upgradeProxy(proxy.address, to.connect(upgrader), { unsafeAllow: ['constructor'], constructorArgs: [], }); const toImplementation = await readImplementationValueFromSlot(proxy.address); - return toImplementation; + + return {proxy, fromImplementation, toImplementation}; } export async function getProtocolVersion( From ecdf7e7a27b0702431781c1d13334c2176c26452 Mon Sep 17 00:00:00 2001 From: Michael Heuer Date: Fri, 21 Jul 2023 17:31:35 +0200 Subject: [PATCH 31/31] fix: added removed args --- packages/contracts/test/test-utils/uups-upgradeable.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/contracts/test/test-utils/uups-upgradeable.ts b/packages/contracts/test/test-utils/uups-upgradeable.ts index b7eed96c4..0d027ea83 100644 --- a/packages/contracts/test/test-utils/uups-upgradeable.ts +++ b/packages/contracts/test/test-utils/uups-upgradeable.ts @@ -97,7 +97,12 @@ export async function ozUpgradeCheckManagingContract( proxy.address ); // Check that upgrade permission is required - await expect(upgrades.upgradeProxy(proxy, to.connect(upgrader))) + await expect( + upgrades.upgradeProxy(proxy.address, to.connect(upgrader), { + unsafeAllow: ['constructor'], + constructorArgs: [], + }) + ) .to.be.revertedWithCustomError(proxy, 'Unauthorized') .withArgs(proxy.address, upgrader.address, upgradePermissionId);