reinitializer number fixed and old interface

aragon · Sep 10, 2024 · 5ce8060 · 5ce8060
1 parent 7b44355
commit 5ce8060
Show file tree

Hide file tree

Showing 6 changed files with 5 additions and 77 deletions.
diff --git a/packages/contracts/src/MajorityVotingBase.sol b/packages/contracts/src/MajorityVotingBase.sol
@@ -218,7 +218,7 @@ abstract contract MajorityVotingBase is
             this.updateMinApprovals.selector ^
             bytes4(
                 keccak256(
-                    "createProposal(bytes,(address,uint256,bytes)[],uint256,bool,bool,uint64,uint64)"
+                    "createProposal(bytes,(address,uint256,bytes)[],uint256,uint64,uint64,uint8,bool)"
                 )
             );
 
@@ -320,8 +320,8 @@ abstract contract MajorityVotingBase is
         returns (bool)
     {
         // In addition to the current IMajorityVoting interface, also support previous version
-        // that did not include the isMinApprovalReached() and minApproval() functions, same
-        // happens with MAJORITY_VOTING_BASE_INTERFACE which did not included updateMinApprovals().
+        // that did not include the `isMinApprovalReached` and `minApproval` functions, same
+        // happens with MAJORITY_VOTING_BASE_INTERFACE which did not include `updateMinApprovals`.
         return
             _interfaceId == MAJORITY_VOTING_BASE_INTERFACE_ID ||
             _interfaceId == MAJORITY_VOTING_BASE_INTERFACE_ID ^ this.updateMinApprovals.selector ||

diff --git a/packages/contracts/src/TokenVoting.sol b/packages/contracts/src/TokenVoting.sol
@@ -23,11 +23,7 @@ contract TokenVoting is IMembership, MajorityVotingBase {
     using SafeCastUpgradeable for uint256;
 
     /// @notice The [ERC-165](https://eips.ethereum.org/EIPS/eip-165) interface ID of the contract.
-    // todo double check there is a strong reason for keeping the initialize function on the interface id
     bytes4 internal constant TOKEN_VOTING_INTERFACE_ID = this.getVotingToken.selector;
-    bytes4 internal constant OLD_TOKEN_VOTING_INTERFACE_ID =
-        bytes4(keccak256("initialize(address,(uint8,uint32,uint32,uint64,uint256),address)")) ^
-            this.getVotingToken.selector;
 
     /// @notice An [OpenZeppelin `Votes`](https://docs.openzeppelin.com/contracts/4.x/api/governance#Votes)
     /// compatible contract referencing the token being used for voting.
@@ -36,20 +32,6 @@ contract TokenVoting is IMembership, MajorityVotingBase {
     /// @notice Thrown if the voting power is zero
     error NoVotingPower();
 
-    error FunctionDeprecated();
-
-    /// @dev Deprecated function.
-    function initialize(
-        IDAO _dao,
-        VotingSettings calldata _votingSettings,
-        IVotesUpgradeable _token
-    ) external initializer {
-        (_dao, _votingSettings, _token);
-
-        // todo TBD should we deprecate this function or only continue with old flow?
-        revert FunctionDeprecated();
-    }
-
     /// @notice Initializes the component.
     /// @dev This method is required to support [ERC-1822](https://eips.ethereum.org/EIPS/eip-1822).
     /// @param _dao The IDAO interface of the associated DAO.
@@ -62,7 +44,7 @@ contract TokenVoting is IMembership, MajorityVotingBase {
         IVotesUpgradeable _token,
         TargetConfig calldata _targetConfig,
         uint256 _minApprovals
-    ) external initializer {
+    ) external reinitializer(2) {
         __MajorityVotingBase_init(_dao, _votingSettings, _targetConfig, _minApprovals);
 
         votingToken = _token;
@@ -88,7 +70,6 @@ contract TokenVoting is IMembership, MajorityVotingBase {
     function supportsInterface(bytes4 _interfaceId) public view virtual override returns (bool) {
         return
             _interfaceId == TOKEN_VOTING_INTERFACE_ID ||
-            _interfaceId == OLD_TOKEN_VOTING_INTERFACE_ID ||
             _interfaceId == type(IMembership).interfaceId ||
             super.supportsInterface(_interfaceId);
     }

diff --git a/packages/contracts/src/TokenVotingSetup.sol b/packages/contracts/src/TokenVotingSetup.sol
@@ -78,10 +78,6 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
     /// @param token The token address
     error TokenNotERC20(address token);
 
-    /// @notice Thrown if passed helpers array is of wrong length.
-    /// @param length The array length of passed helpers.
-    error WrongHelpersArrayLength(uint256 length);
-
     /// @notice The contract constructor deploying the plugin implementation contract
     /// and receiving the governance token base contracts to clone from.
     /// @param _governanceERC20Base The base `GovernanceERC20` contract to create clones from.

diff --git a/packages/contracts/test/10_unit-testing/11_plugin.ts b/packages/contracts/test/10_unit-testing/11_plugin.ts
@@ -270,23 +270,6 @@ describe('TokenVoting', function () {
       ).to.be.revertedWith('Initializable: contract is already initialized');
     });
 
-    it('reverts if using DEPRECATED intialize function', async () => {
-      const {dao, uninitializedPlugin, defaultVotingSettings, token} =
-        await loadFixture(globalFixture);
-
-      // Try to call deprecated function (previous function with no minApproval param)
-      await expect(
-        uninitializedPlugin[INITIALIZE_SIGNATURE_OLD](
-          dao.address,
-          defaultVotingSettings,
-          token.address
-        )
-      ).to.be.revertedWithCustomError(
-        uninitializedPlugin,
-        'FunctionDeprecated'
-      );
-    });
-
     it('emits the `MembershipContractAnnounced` event', async () => {
       const {
         dao,

diff --git a/packages/contracts/test/10_unit-testing/12_plugin-setup.ts b/packages/contracts/test/10_unit-testing/12_plugin-setup.ts
@@ -842,35 +842,6 @@ describe('TokenVotingSetup', function () {
   });
 
   describe('prepareUninstallation', async () => {
-    // TODO: this test will need to be removed...
-    it.only('fails when the wrong number of helpers is supplied', async () => {
-      const {pluginSetup, dao, prepareUninstallationInputs} = await loadFixture(
-        fixture
-      );
-
-      const plugin = ethers.Wallet.createRandom().address;
-
-      await expect(
-        pluginSetup.prepareUninstallation(dao.address, {
-          plugin,
-          currentHelpers: [],
-          data: prepareUninstallationInputs,
-        })
-      )
-        .to.be.revertedWithCustomError(pluginSetup, 'WrongHelpersArrayLength')
-        .withArgs(0);
-
-      await expect(
-        pluginSetup.prepareUninstallation(dao.address, {
-          plugin,
-          currentHelpers: [AddressZero, AddressZero, AddressZero],
-          data: prepareUninstallationInputs,
-        })
-      )
-        .to.be.revertedWithCustomError(pluginSetup, 'WrongHelpersArrayLength')
-        .withArgs(3);
-    });
-
     it('correctly returns permissions, when the required number of helpers is supplied', async () => {
       const {
         deployer,

diff --git a/packages/contracts/test/test-utils/token-voting-constants.ts b/packages/contracts/test/test-utils/token-voting-constants.ts
@@ -1,7 +1,6 @@
 import {ethers} from 'hardhat';
 
 export const TOKEN_VOTING_INTERFACE = new ethers.utils.Interface([
-  'function initialize(address,tuple(uint8,uint32,uint32,uint64,uint256),address)',
   'function getVotingToken()',
 ]);
 
@@ -36,9 +35,7 @@ export const VOTING_EVENTS = {
 
 export const ANY_ADDR = '0xFFfFfFffFFfffFFfFFfFFFFFffFFFffffFfFFFfF';
 
-export const INITIALIZE_SIGNATURE_OLD =
-  'initialize(address,(uint8,uint32,uint32,uint64,uint256),address)';
-
+
 export const INITIALIZE_SIGNATURE =
   'initialize(address,(uint8,uint32,uint32,uint64,uint256),address,(address,uint8),uint256)';