WIP 🚧 feat: token voting target improvements #30

clauBv23 · 2024-09-04T07:51:55Z

No description provided.

packages/contracts/hardhat.config.ts

clauBv23 · 2024-09-13T15:25:06Z

packages/contracts/src/TokenVotingSetup.sol

+        permissions[1] = PermissionLib.MultiTargetPermission(
+            PermissionLib.Operation.Revoke,
+            _payload.plugin,
+            _dao,
+            PermissionLib.NO_CONDITION,
+            UPGRADE_PLUGIN_PERMISSION_ID
+        );


from v1.3 the UPGRADE_PLUGIN_PERMISSION_ID is not being granted to the DAO, so it doesn’t need to be revoked when uninstalling.
It is already being revoked when upgrading from a previous version

done in 6b02e12

clauBv23 · 2024-09-13T15:37:02Z

packages/contracts/src/TokenVotingSetup.sol

+        permissions[4] = PermissionLib.MultiTargetPermission(
+            PermissionLib.Operation.Revoke,
+            _payload.plugin,
+            address(type(uint160).max), // ANY_ADDR
+            PermissionLib.NO_CONDITION,
+            TokenVoting(IMPLEMENTATION).CREATE_PROPOSAL_PERMISSION_ID()
+        );


In the previous version, after uninstalling a plugin, the proposal creation and voting process were still allowed, even though proposal actions would fail due to the plugin losing execute permission, but, revoking CREATE_PROPOSAL_PERMISSION now changes this behavior.

I'm not sure if this would create any significant issues in practice, but one potential scenario that comes to mind is users using an uninstalled token voting plugin for off-chain decisions. In such cases, they might still create proposals without actions, using the proposal description as metadata. While this might not be a very common use case, it seems worth considering. wdyt?

We agreed on not allowing the proposal creation once the plugin is uninstalled. In the end, the functionality of a plugin is to create proposals on the Dao it is installed on, so when it is uninstalled it should not allow proposal creation.

However, users can always grant the create proposal permission after uninstallation if they need it

Co-authored-by: Jør∂¡ <[email protected]>

add executor interface

bytes param added

…ter-ivote-checks

ivotes checks

…-tests better upgrade tests

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

Co-authored-by: Jør∂¡ <[email protected]>

metadata extension

brickpop · 2024-10-08T16:44:36Z

packages/contracts/src/MajorityVotingBase.sol

@@ -259,6 +271,10 @@ abstract contract MajorityVotingBase is
    /// @param proposalId The ID of the proposal.
    error ProposalExecutionForbidden(uint256 proposalId);

+    /// @notice Thrown if the proposal with same actions and metadata already exists.
+    /// @param proposalId The id of the proposal.
+    error ProposalAlreadyExists(uint256 proposalId);


Same as with multisig

aragon#19 (comment)

brickpop · 2024-10-08T16:46:35Z

packages/contracts/src/MajorityVotingBase.sol

        );
+
+        emit ProposalExecuted(_proposalId);


I can't see it being removed from the plugin side. Just to confirm

the event is defined in IProposal and emitted on each plugin implementation of IProposal

brickpop · 2024-10-08T16:50:18Z

packages/contracts/src/TokenVoting.sol

+        TargetConfig calldata _targetConfig,
+        uint256 _minApprovals,
+        bytes calldata _pluginMetadata
+    ) external onlyCallAtInitialization reinitializer(2) {


Just to confirm, aren't we initializing build 3 at this point?

Suggested change

) external onlyCallAtInitialization reinitializer(2) {

) external onlyCallAtInitialization reinitializer(3) {

brickpop · 2024-10-08T16:51:12Z

packages/contracts/src/TokenVoting.sol

+    /// @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) {


Same question

brickpop · 2024-10-08T16:56:14Z

packages/contracts/src/TokenVoting.sol

+        Action[] calldata _actions,
+        bytes memory _metadata
+    ) public pure override returns (uint256) {
+        return uint256(keccak256(abi.encode(_actions, _metadata)));


Same as with multisig

brickpop · 2024-10-08T16:57:35Z

packages/contracts/src/TokenVotingSetup.sol

+            PluginUUPSUpgradeable.TargetConfig memory targetConfig,
+            uint256 minApprovals,
+            bytes memory pluginMetadata
        ) = abi.decode(
                _data,
                (
                    MajorityVotingBase.VotingSettings,
                    TokenSettings,
                    GovernanceERC20.MintSettings,
-                    uint256
+                    PluginUUPSUpgradeable.TargetConfig,
+                    uint256,
+                    bytes
                )


Same as with multisig. Let's encode and decode data using a standard mechanism:

https://github.com/aragon/taiko-contracts/blob/main/src/setup/MultisigPluginSetup.sol#L30C13-L30C41
https://github.com/aragon/taiko-contracts/blob/main/src/setup/MultisigPluginSetup.sol#L101-L117

Might be postponed if needed

brickpop · 2024-10-08T17:00:17Z

packages/contracts/src/TokenVotingSetup.sol

+        permissions[2] = PermissionLib.MultiTargetPermission(
+            PermissionLib.Operation.GrantWithCondition,
+            plugin,
+            address(type(uint160).max), // ANY_ADDR


Same as with multisig, if feasible

brickpop · 2024-10-08T17:03:54Z

packages/contracts/src/TokenVotingSetup.sol

+        );
+        (bool success2, bytes memory data2) = token.staticcall(
+            abi.encodeWithSelector(IVotesUpgradeable.getVotes.selector, address(this))
+        );
+        (bool success3, bytes memory data3) = token.staticcall(
+            abi.encodeWithSelector(IVotesUpgradeable.getPastVotes.selector, address(this), 0)
+        );
+
+        return (success1 &&
+            data1.length == 0x20 &&
+            success2 &&
+            data2.length == 0x20 &&
+            success3 &&
+            data3.length == 0x20);


No need to compute everything upfront. Lazy evaluation:

if (no) return false; else if(no) return false; else if(no) return false return true;

* fix tests and contract * change * add actions in proposal id generation * rename'

Rekard0 · 2024-10-15T12:08:33Z

packages/contracts/src/MajorityVotingBase.sol

        uint256 allowFailureMap;
        uint256 minApprovalPower;
+        TargetConfig targetConfig; // added in v1.3


you mean added in v1.4 ?

clauBv23 and others added 6 commits August 30, 2024 17:16

feat: add condition

cb368a3

feat: add new params to token voting

6173c49

add target configs permanently and fix tests

7b44355

reinitializer number fixed and old interface

5ce8060

remove todo comments

84ef2cd

initializeFrom with build number and data

207d988

clauBv23 commented Sep 13, 2024

View reviewed changes

packages/contracts/hardhat.config.ts Outdated Show resolved Hide resolved

clauBv23 commented Sep 13, 2024

View reviewed changes

novaknole and others added 21 commits September 17, 2024 13:53

ivotes checks

5dfccfc

add initialization modifier for safety + improved targetconfig

b2eef30

bytes param added

96bf27b

rename param function

cd5543e

Update packages/contracts/src/TokenVoting.sol

42073e3

Co-authored-by: Jør∂¡ <[email protected]>

Update packages/contracts/src/TokenVoting.sol

2051e27

Co-authored-by: Jør∂¡ <[email protected]>

fix typo

70f28e6

add delegate call

ccfe8ec

silence warnings

f3cb27c

add executor interface

386d417

action as free level

329e76b

Merge pull request #34 from aragon/feature/executors

283c2c6

add executor interface

Merge pull request #32 from aragon/feature/bytes-param-add

fba0116

bytes param added

remove unneccessary revoke

6b02e12

Merge branch 'feat/token-voting-target-improvements' into feature/bet…

4a8be44

…ter-ivote-checks

Merge pull request #31 from aragon/feature/better-ivote-checks

41b5d7f

ivotes checks

better upgrade tests

93e6d31

remove hardcoded value

5178b67

Merge pull request #35 from aragon/feature/add-initializeFrom-upgrade…

4780717

…-tests better upgrade tests

callinitialization from osx-commons

2690267

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

630bfd9

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

novaknole and others added 11 commits September 29, 2024 18:08

fix tohex

0647d5d

metadata extension

4f86c7c

Update packages/contracts/src/MajorityVotingBase.sol

f316675

Co-authored-by: Jør∂¡ <[email protected]>

Update packages/contracts/src/TokenVoting.sol

ccfa5d9

Co-authored-by: Jør∂¡ <[email protected]>

Update packages/contracts/src/TokenVoting.sol

d49093c

Co-authored-by: Jør∂¡ <[email protected]>

Update packages/contracts/src/TokenVoting.sol

ab1af36

Co-authored-by: Jør∂¡ <[email protected]>

Update packages/contracts/src/TokenVotingSetup.sol

3176cd4

Co-authored-by: Jør∂¡ <[email protected]>

Update packages/contracts/src/TokenVotingSetup.sol

3de9e37

Co-authored-by: Jør∂¡ <[email protected]>

Update packages/contracts/src/TokenVotingSetup.sol

9799f4e

Co-authored-by: Jør∂¡ <[email protected]>

permission added

7b3d080

Merge pull request #36 from aragon/feature/metadata

ba6dcf8

metadata extension

brickpop reviewed Oct 8, 2024

View reviewed changes

novaknole added 2 commits October 9, 2024 17:22

metadata rename

4e5d291

fix tests and contract (#37)

a857d9e

* fix tests and contract * change * add actions in proposal id generation * rename'

Rekard0 reviewed Oct 15, 2024

View reviewed changes

novaknole deleted the branch feat/add-minimal-approval-configuration October 18, 2024 06:49

novaknole closed this Oct 18, 2024

Rekard0 mentioned this pull request Oct 23, 2024

Feat/token voting target improvements #38

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP 🚧 feat: token voting target improvements #30

WIP 🚧 feat: token voting target improvements #30

clauBv23 commented Sep 4, 2024

clauBv23 Sep 13, 2024

clauBv23 Oct 14, 2024

clauBv23 Sep 13, 2024

clauBv23 Oct 14, 2024

brickpop Oct 8, 2024

brickpop Oct 8, 2024

clauBv23 Oct 14, 2024

brickpop Oct 8, 2024

brickpop Oct 8, 2024

brickpop Oct 8, 2024

brickpop Oct 8, 2024

brickpop Oct 8, 2024

brickpop Oct 8, 2024

Rekard0 Oct 15, 2024

	) external onlyCallAtInitialization reinitializer(2) {
	) external onlyCallAtInitialization reinitializer(3) {

WIP 🚧 feat: token voting target improvements #30

WIP 🚧 feat: token voting target improvements #30

Conversation

clauBv23 commented Sep 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment