feat: add auth to execute() and update Setup & testing

packages/contracts/src/MajorityVotingBase.sol

@@ -229,9 +229,13 @@ abstract contract MajorityVotingBase is
     bytes32 public constant UPDATE_VOTING_SETTINGS_PERMISSION_ID =
         keccak256("UPDATE_VOTING_SETTINGS_PERMISSION");
 
-    /// @notice The ID of the permission required to call the `addAddresses` and `removeAddresses` functions.
+    /// @notice The ID of the permission required to call the `createProposal` functions.
     bytes32 public constant CREATE_PROPOSAL_PERMISSION_ID = keccak256("CREATE_PROPOSAL_PERMISSION");
 
+    /// @notice The ID of the permission required to call the `execute` function.
+    bytes32 public constant EXECUTE_PROPOSAL_PERMISSION_ID =
+        keccak256("EXECUTE_PROPOSAL_PERMISSION");
+
     /// @notice A mapping between proposal IDs and proposal information.
     // solhint-disable-next-line named-parameters-mapping
     mapping(uint256 => Proposal) internal proposals;
@@ -362,7 +366,7 @@ abstract contract MajorityVotingBase is
     }
 
     /// @inheritdoc IMajorityVoting
-    function execute(uint256 _proposalId) public virtual {
+    function execute(uint256 _proposalId) public virtual auth(EXECUTE_PROPOSAL_PERMISSION_ID) {
         if (!_canExecute(_proposalId)) {
             revert ProposalExecutionForbidden(_proposalId);
         }

packages/contracts/src/TokenVoting.sol

@@ -20,7 +20,7 @@
 /// @notice The majority voting implementation using an
 /// [OpenZeppelin `Votes`](https://docs.openzeppelin.com/contracts/4.x/api/governance#Votes)
 /// compatible governance token.
 /// @dev v1.3 (Release 1, Build 3). For each upgrade, if the reinitialization step is required, increment the version numbers in the modifier for both the initialize and initializeFrom functions.
 /// @custom:security-contact [email protected]
 contract TokenVoting is IMembership, MajorityVotingBase {
     using SafeCastUpgradeable for uint256;
@@ -62,10 +62,10 @@
         emit MembershipContractAnnounced({definingContract: address(_token)});
     }
 
     /// @notice Reinitializes the TokenVoting after an upgrade from a previous protocol version.For each reinitialization step, use the `_fromBuild` version to decide which internal functions to call for reinitialization.
     /// @dev WARNING: The contract should only be upgradeable through PSP to ensure that _fromBuild is not incorrectly passed, and that the appropriate permissions for the upgrade are properly configured.
     /// @param _fromBuild The build version number of the previous implementation contract this upgrade is transitioning from.
     /// @param _initData The initialization data to be passed to via `upgradeToAndCall` (see [ERC-1967](https://docs.openzeppelin.com/contracts/4.x/api/proxy#ERC1967Upgrade)).
     function initializeFrom(uint16 _fromBuild, bytes calldata _initData) external reinitializer(2) {
         if (_fromBuild < 3) {
             (
@@ -224,7 +224,7 @@
     }
 
     /// @inheritdoc MajorityVotingBase
     function _vote(
         uint256 _proposalId,
         VoteOption _voteOption,
         address _voter,
@@ -263,7 +263,14 @@
             votingPower: votingPower
         });
 
-        if (_tryEarlyExecution && _canExecute(_proposalId)) {
+        if (!_tryEarlyExecution) {
+            return;
+        }
+
+        if (
+            _canExecute(_proposalId) &&
+            dao().hasPermission(address(this), _voter, EXECUTE_PROPOSAL_PERMISSION_ID, _msgData())
+        ) {
             _execute(_proposalId);
         }
     }

packages/contracts/src/TokenVotingSetup.sol

@@ -49,6 +49,13 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
     /// @notice The ID of the permission required to call the `upgradeToAndCall` function.
     bytes32 internal constant UPGRADE_PLUGIN_PERMISSION_ID = keccak256("UPGRADE_PLUGIN_PERMISSION");
 
+    /// @notice The ID of the permission required to call the `execute` function.
+    bytes32 internal constant EXECUTE_PROPOSAL_PERMISSION_ID =
+        keccak256("EXECUTE_PROPOSAL_PERMISSION");
+
+    /// @notice A special address encoding permissions that are valid for any address `who` or `where`.
+    address internal constant ANY_ADDR = address(type(uint160).max);
+
     /// @notice The address of the `TokenVoting` base contract.
     // solhint-disable-next-line immutable-vars-naming
     TokenVoting private immutable tokenVotingBase;
@@ -175,7 +182,7 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
         // Prepare permissions
         PermissionLib.MultiTargetPermission[]
             memory permissions = new PermissionLib.MultiTargetPermission[](
-                tokenSettings.addr != address(0) ? 5 : 6
+                tokenSettings.addr != address(0) ? 6 : 7
             );
 
         // Set plugin permissions to be granted.
@@ -200,7 +207,7 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
         permissions[2] = PermissionLib.MultiTargetPermission(
             PermissionLib.Operation.GrantWithCondition,
             plugin,
-            address(type(uint160).max), // ANY_ADDR
+            ANY_ADDR,
             preparedSetupData.helpers[0], // VotingPowerCondition
             TokenVoting(IMPLEMENTATION).CREATE_PROPOSAL_PERMISSION_ID()
         );
@@ -221,10 +228,18 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
             permissionId: SET_METADATA_PERMISSION_ID
         });
 
+        permissions[5] = PermissionLib.MultiTargetPermission({
+            operation: PermissionLib.Operation.Grant,
+            where: plugin,
+            who: ANY_ADDR,
+            condition: PermissionLib.NO_CONDITION,
+            permissionId: EXECUTE_PROPOSAL_PERMISSION_ID
+        });
+
         if (tokenSettings.addr == address(0)) {
             bytes32 tokenMintPermission = GovernanceERC20(token).MINT_PERMISSION_ID();
 
-            permissions[5] = PermissionLib.MultiTargetPermission({
+            permissions[6] = PermissionLib.MultiTargetPermission({
                 operation: PermissionLib.Operation.Grant,
                 where: token,
                 who: _dao,
@@ -251,7 +266,7 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
             address votingPowerCondition = address(new VotingPowerCondition(_payload.plugin));
 
             PermissionLib.MultiTargetPermission[]
-                memory permissions = new PermissionLib.MultiTargetPermission[](4);
+                memory permissions = new PermissionLib.MultiTargetPermission[](5);
 
             permissions[0] = PermissionLib.MultiTargetPermission({
                 operation: PermissionLib.Operation.Revoke,
@@ -264,7 +279,7 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
             permissions[1] = PermissionLib.MultiTargetPermission(
                 PermissionLib.Operation.GrantWithCondition,
                 _payload.plugin,
-                address(type(uint160).max), // ANY_ADDR
+                ANY_ADDR, // ANY_ADDR
                 votingPowerCondition,
                 TokenVoting(IMPLEMENTATION).CREATE_PROPOSAL_PERMISSION_ID()
             );
@@ -285,6 +300,14 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
                 permissionId: SET_METADATA_PERMISSION_ID
             });
 
+            permissions[4] = PermissionLib.MultiTargetPermission({
+                operation: PermissionLib.Operation.Grant,
+                where: _payload.plugin,
+                who: ANY_ADDR,
+                condition: PermissionLib.NO_CONDITION,
+                permissionId: EXECUTE_PROPOSAL_PERMISSION_ID
+            });
+
             preparedSetupData.permissions = permissions;
             preparedSetupData.helpers = new address[](1);
             preparedSetupData.helpers[0] = votingPowerCondition;
@@ -299,7 +322,7 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
         SetupPayload calldata _payload
     ) external view returns (PermissionLib.MultiTargetPermission[] memory permissions) {
         // Prepare permissions.
-        permissions = new PermissionLib.MultiTargetPermission[](5);
+        permissions = new PermissionLib.MultiTargetPermission[](6);
 
         // Set permissions to be Revoked.
         permissions[0] = PermissionLib.MultiTargetPermission({
@@ -337,10 +360,18 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
         permissions[4] = PermissionLib.MultiTargetPermission({
             operation: PermissionLib.Operation.Revoke,
             where: _payload.plugin,
-            who: address(type(uint160).max), // ANY_ADDR
+            who: ANY_ADDR, // ANY_ADDR
             condition: PermissionLib.NO_CONDITION,
             permissionId: TokenVoting(IMPLEMENTATION).CREATE_PROPOSAL_PERMISSION_ID()
         });
+
+        permissions[5] = PermissionLib.MultiTargetPermission({
+            operation: PermissionLib.Operation.Revoke,
+            where: _payload.plugin,
+            who: ANY_ADDR,
+            condition: PermissionLib.NO_CONDITION,
+            permissionId: EXECUTE_PROPOSAL_PERMISSION_ID
+        });
     }
 
     /// @notice Unsatisfiably determines if the token is an IVotes interface.

packages/contracts/test/10_unit-testing/11_plugin.ts

@@ -27,6 +27,7 @@ import {
 import {
   TOKEN_VOTING_INTERFACE,
   UPDATE_VOTING_SETTINGS_PERMISSION_ID,
+  EXECUTE_PROPOSAL_PERMISSION_ID,
   INITIALIZE_SIGNATURE,
   INITIALIZE_SIGNATURE_OLD,
   Operation,
@@ -198,6 +199,11 @@ async function globalFixture(): Promise<GlobalFixtureResult> {
     deployer
   );
 
+  // Grant ANY_ADDR the permission to execute proposals
+  await dao
+    .connect(deployer)
+    .grant(initializedPlugin.address, ANY_ADDR, EXECUTE_PROPOSAL_PERMISSION_ID);
+
   // Grant deployer the permission to update the voting settings
   await dao
     .connect(deployer)
@@ -2148,6 +2154,71 @@ describe('TokenVoting', function () {
           .to.be.revertedWithCustomError(plugin, 'ProposalExecutionForbidden')
           .withArgs(id);
       });
+
+      it('can not execute even if participation and support are met when caller does not have permission', async () => {
+        const {
+          alice,
+          bob,
+          carol,
+          dave,
+          eve,
+          frank,
+          grace,
+          initializedPlugin: plugin,
+          dummyMetadata,
+          dummyActions,
+          dao,
+        } = await loadFixture(localFixture);
+
+        const endDate = (await time.latest()) + TIME.DAY;
+        const id = await createProposalId(
+          plugin.address,
+          dummyActions,
+          dummyMetadata
+        );
+
+        // Create a proposal.
+        await plugin[CREATE_PROPOSAL_SIGNATURE](
+          dummyMetadata,
+          dummyActions,
+          0,
+          0,
+          endDate,
+          VoteOption.None,
+          false
+        );
+
+        // Vote with enough voters so that the execution criteria are met.
+        await voteWithSigners(plugin, id, {
+          yes: [alice, bob, carol], // 30 votes
+          no: [dave, eve], // 20 votes
+          abstain: [frank, grace], // 20 votes
+        });
+
+        // Wait until the vote is over.
+        await time.increaseTo(endDate);
+
+        // Check that the proposal can be executed.
+        expect(await plugin.isSupportThresholdReached(id)).to.be.true;
+        expect(await plugin.isMinParticipationReached(id)).to.be.true;
+        expect(await plugin.canExecute(id)).to.equal(true);
+
+        // Revoke execute permission from ANY_ADDR
+        await dao.revoke(
+          plugin.address,
+          ANY_ADDR,
+          EXECUTE_PROPOSAL_PERMISSION_ID
+        );
+
+        await expect(plugin.connect(alice).execute(id))
+          .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized')
+          .withArgs(
+            dao.address,
+            plugin.address,
+            alice.address,
+            EXECUTE_PROPOSAL_PERMISSION_ID
+          );
+      });
     });
 
     describe('Early Execution', async () => {
@@ -2646,6 +2717,65 @@ describe('TokenVoting', function () {
           .to.be.revertedWithCustomError(plugin, 'ProposalExecutionForbidden')
           .withArgs(id);
       });
+
+      it('record vote correctly without executing even when tryEarlyExecution options is selected', async () => {
+        const {
+          alice,
+          bob,
+          carol,
+          dave,
+          eve,
+          frank,
+          grace,
+          dao,
+          initializedPlugin: plugin,
+          dummyMetadata,
+          dummyActions,
+        } = await loadFixture(localFixture);
+
+        // Create a Proposal.
+        const endDate = (await time.latest()) + TIME.DAY;
+        const id = await createProposalId(
+          plugin.address,
+          dummyActions,
+          dummyMetadata
+        );
+
+        await plugin[CREATE_PROPOSAL_SIGNATURE](
+          dummyMetadata,
+          dummyActions,
+          0,
+          0,
+          endDate,
+          VoteOption.None,
+          false
+        );
+
+        // Vote 40 votes for `Yes`. The proposal can still get defeated if the remaining 60 votes vote for `No`.
+        await voteWithSigners(plugin, id, {
+          yes: [alice, bob, carol, dave, eve], // 50 votes
+          no: [], // 0 votes
+          abstain: [], // 0 votes
+        });
+
+        // Check that the proposal cannot be early executed and didn't execute yet.
+        expect((await plugin.getProposal(id)).executed).to.equal(false);
+        expect(await plugin.canExecute(id)).to.equal(false);
+
+        // Revoke execute permission from ANY_ADDR
+        await dao.revoke(
+          plugin.address,
+          ANY_ADDR,
+          EXECUTE_PROPOSAL_PERMISSION_ID
+        );
+
+        // Vote `Yes` with Frank with `tryEarlyExecution` being turned on.
+        // The vote is decided now, but proposal should not be executed yet.
+        await plugin.connect(frank).vote(id, VoteOption.Yes, true);
+        // Check that the proposal can be executed but didn't execute yet.
+        expect((await plugin.getProposal(id)).executed).to.equal(false);
+        expect(await plugin.canExecute(id)).to.equal(true);
+      });
     });
 
     describe('Vote Replacement', async () => {