Merge branch 'feat/token-voting-target-improvements' of https://githu…

…b.com/aragon/token-voting-plugin into feat/token-voting-target-improvements

packages/contracts/test/10_unit-testing/12_plugin-setup.ts

@@ -10,6 +10,8 @@ import {
   IERC20Upgradeable__factory,
   IVotesUpgradeable__factory,
 } from '../../typechain';
+import {plugins} from '../../typechain/@aragon/osx-v1.0.0';
+import {IGovernanceWrappedERC20__factory} from '../../typechain/factories/src/ERC20/governance';
 import {MajorityVotingBase} from '../../typechain/src/MajorityVotingBase';
 import {
   ANY_ADDR,
@@ -41,8 +43,6 @@ import {loadFixture} from '@nomicfoundation/hardhat-network-helpers';
 import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers';
 import {expect} from 'chai';
 import {ethers} from 'hardhat';
-import { plugins } from '../../typechain/@aragon/osx-v1.0.0';
-import { IGovernanceWrappedERC20__factory } from '../../typechain/factories/src/ERC20/governance';
 
 const abiCoder = ethers.utils.defaultAbiCoder;
 const AddressZero = ethers.constants.AddressZero;
@@ -375,8 +375,8 @@ describe('TokenVotingSetup', function () {
         preparedSetupData: {helpers, permissions},
       } = await pluginSetup.callStatic.prepareInstallation(dao.address, data);
 
-      expect(await pluginSetup.supportsIVotesInterface(erc20.address))
-        .to.be.false;
+      expect(await pluginSetup.supportsIVotesInterface(erc20.address)).to.be
+        .false;
 
       expect(plugin).to.be.equal(anticipatedPluginAddress);
       expect(helpers.length).to.be.equal(2);
@@ -466,8 +466,8 @@ describe('TokenVotingSetup', function () {
         erc20.address
       );
 
-      expect(await pluginSetup.supportsIVotesInterface(erc20.address))
-        .to.be.false;
+      expect(await pluginSetup.supportsIVotesInterface(erc20.address)).to.be
+        .false;
 
       // If a token address is not passed, it must have deployed GovernanceERC20.
       const ivotesInterfaceId = getInterfaceId(
@@ -544,9 +544,8 @@ describe('TokenVotingSetup', function () {
         preparedSetupData: {helpers, permissions},
       } = await pluginSetup.callStatic.prepareInstallation(dao.address, data);
 
-      expect(
-        await pluginSetup.supportsIVotesInterface(governanceERC20.address)
-      ).to.be.true;
+      expect(await pluginSetup.supportsIVotesInterface(governanceERC20.address))
+        .to.be.true;
 
       expect(plugin).to.be.equal(anticipatedPluginAddress);
       expect(helpers.length).to.be.equal(2);
@@ -588,9 +587,12 @@ describe('TokenVotingSetup', function () {
     });
 
     it('correctly returns plugin, helpers and permissions, when a token address is not supplied', async () => {
-      const {pluginSetup, dao, defaultTokenSettings, prepareInstallationInputs} = await loadFixture(
-        fixture
-      );
+      const {
+        pluginSetup,
+        dao,
+        defaultTokenSettings,
+        prepareInstallationInputs,
+      } = await loadFixture(fixture);
 
       const nonce = await ethers.provider.getTransactionCount(
         pluginSetup.address
@@ -619,9 +621,7 @@ describe('TokenVotingSetup', function () {
       );
 
       expect(
-        await pluginSetup.supportsIVotesInterface(
-          defaultTokenSettings.addr
-        )
+        await pluginSetup.supportsIVotesInterface(defaultTokenSettings.addr)
       ).to.be.false;
 
       expect(plugin).to.be.equal(anticipatedPluginAddress);
@@ -756,10 +756,8 @@ describe('TokenVotingSetup', function () {
         IERC20Upgradeable__factory.createInterface()
       );
 
-      expect(await token.supportsInterface(ivotesInterfaceId))
-        .to.be.true;
-      expect(await token.supportsInterface(iERC20InterfaceId))
-        .to.be.true;
+      expect(await token.supportsInterface(ivotesInterfaceId)).to.be.true;
+      expect(await token.supportsInterface(iERC20InterfaceId)).to.be.true;
     });
   });
 
@@ -924,6 +922,31 @@ describe('TokenVotingSetup', function () {
         ],
       ]);
     });
+
+    it('returns the permissions expected for the update from build 3 (empty list)', async () => {
+      const {pluginSetup, dao, prepareUpdateBuild3Inputs} = await loadFixture(
+        fixture
+      );
+      const plugin = ethers.Wallet.createRandom().address;
+
+      // Make a static call to check that the plugin update data being returned is correct.
+      const {
+        initData: initData,
+        preparedSetupData: {helpers, permissions},
+      } = await pluginSetup.callStatic.prepareUpdate(dao.address, 3, {
+        currentHelpers: [
+          ethers.Wallet.createRandom().address,
+          ethers.Wallet.createRandom().address,
+        ],
+        data: prepareUpdateBuild3Inputs,
+        plugin,
+      });
+
+      // Check the return data. There should be no permission needed for build 3.
+      expect(initData).to.be.eq('0x');
+      expect(permissions.length).to.be.equal(0);
+      expect(helpers.length).to.be.equal(0);
+    });
   });
 
   describe('prepareUninstallation', async () => {

packages/contracts/test/10_unit-testing/base/11_majority-voting.ts

@@ -19,6 +19,7 @@ import {
   Operation,
   SET_TARGET_CONFIG_PERMISSION_ID,
   TargetConfig,
+  UPDATE_VOTING_SETTINGS_PERMISSION_ID,
 } from '../../test-utils/token-voting-constants';
 import {IMajorityVoting_V1_3_0__factory} from '../../test-utils/typechain-versions';
 import {VotingMode} from '../../test-utils/voting-helpers';
@@ -174,6 +175,22 @@ describe('MajorityVotingMock', function () {
       );
     });
 
+    it('reverts if caller is unauthorized', async () => {
+      const unauthorizedAddr = signers[5];
+      await expect(
+        votingBase
+          .connect(unauthorizedAddr)
+          .updateVotingSettings(votingSettings)
+      )
+        .to.be.revertedWithCustomError(votingBase, 'DaoUnauthorized')
+        .withArgs(
+          dao.address,
+          votingBase.address,
+          unauthorizedAddr.address,
+          UPDATE_VOTING_SETTINGS_PERMISSION_ID
+        );
+    });
+
     it('reverts if the support threshold specified equals 100%', async () => {
       votingSettings.supportThreshold = pctToRatio(100);
       await expect(votingBase.updateVotingSettings(votingSettings))
@@ -240,6 +257,20 @@ describe('MajorityVotingMock', function () {
       );
     });
 
+    it('reverts if caller is unauthorized', async () => {
+      const unauthorizedAddr = signers[5];
+      await expect(
+        votingBase.connect(unauthorizedAddr).updateMinApprovals(pctToRatio(10))
+      )
+        .to.be.revertedWithCustomError(votingBase, 'DaoUnauthorized')
+        .withArgs(
+          dao.address,
+          votingBase.address,
+          unauthorizedAddr.address,
+          UPDATE_VOTING_SETTINGS_PERMISSION_ID
+        );
+    });
+
     it('reverts if the minimum approval specified exceeds 100%', async () => {
       minApproval = pctToRatio(1000);
 

packages/contracts/test/20_integration-testing/22_setup-processing.ts

@@ -2,7 +2,11 @@ import {METADATA, VERSION} from '../../plugin-settings';
 import {GovernanceERC20} from '../../typechain';
 import {MajorityVotingBase} from '../../typechain/src/MajorityVotingBase';
 import {getProductionNetworkName, findPluginRepo} from '../../utils/helpers';
-import {Operation, TargetConfig} from '../test-utils/token-voting-constants';
+import {
+  Operation,
+  TargetConfig,
+  latestInitializerVersion,
+} from '../test-utils/token-voting-constants';
 import {
   GovernanceERC20__factory,
   TokenVotingSetup,
@@ -366,7 +370,8 @@ describe(`PluginSetup processing on network '${productionNetworkName}'`, functio
       pluginSetupRefLatestBuild,
       1,
       Object.values(prepareInstallData),
-      prepareUpdateData
+      prepareUpdateData,
+      latestInitializerVersion
     );
   });
 
@@ -390,7 +395,8 @@ describe(`PluginSetup processing on network '${productionNetworkName}'`, functio
       pluginSetupRefLatestBuild,
       2,
       Object.values(prepareInstallData),
-      prepareUpdateData
+      prepareUpdateData,
+      latestInitializerVersion
     );
   });
 });

packages/contracts/test/20_integration-testing/test-helpers.ts

@@ -6,6 +6,7 @@ import {
 } from '../../typechain';
 import {ProxyCreatedEvent} from '../../typechain/@aragon/osx-commons-contracts/src/utils/deployment/ProxyFactory';
 import {PluginUUPSUpgradeable__factory} from '../../typechain/factories/@aragon/osx-v1.0.0/core/plugin';
+import {latestPluginBuild} from '../test-utils/token-voting-constants';
 import {
   DAO_PERMISSIONS,
   PLUGIN_SETUP_PROCESSOR_PERMISSIONS,
@@ -27,7 +28,7 @@ import {expect} from 'chai';
 import {ContractTransaction} from 'ethers';
 import {ethers} from 'hardhat';
 
-const latestBuild = 3;
+const OZ_INITIALIZED_SLOT_POSITION = 0;
 
 export async function installPLugin(
   signer: SignerWithAddress,
@@ -244,7 +245,8 @@ export async function updateFromBuildTest(
   pluginSetupRefLatestBuild: PluginSetupProcessorStructs.PluginSetupRefStruct,
   build: number,
   installationInputs: any[],
-  updateInputs: any[]
+  updateInputs: any[],
+  reinitializedVersion: number
 ) {
   // Grant deployer all required permissions
   await dao
@@ -326,7 +328,7 @@ export async function updateFromBuildTest(
       pluginSetupRefLatestBuild,
       ethers.utils.defaultAbiCoder.encode(
         getNamedTypesFromMetadata(
-          METADATA.build.pluginSetup.prepareUpdate[latestBuild].inputs
+          METADATA.build.pluginSetup.prepareUpdate[latestPluginBuild].inputs
         ),
         updateInputs
       )
@@ -344,6 +346,16 @@ export async function updateFromBuildTest(
       deployer
     ).implementation();
   expect(await plugin.implementation()).to.equal(implementationLatestBuild);
+
+  // check the plugin was reinitialized, OZs `_initialized` at storage slot [0] is correct
+  expect(
+    ethers.BigNumber.from(
+      await ethers.provider.getStorageAt(
+        plugin.address,
+        OZ_INITIALIZED_SLOT_POSITION
+      )
+    ).toNumber()
+  ).to.equal(reinitializedVersion);
 }
 
 // TODO Move into OSX commons as part of Task OS-928.