From e35f6f8737f2032a419de3646faf5e82115de8de Mon Sep 17 00:00:00 2001 From: Michael Heuer <20623991+Michael-A-Heuer@users.noreply.github.com> Date: Wed, 4 Oct 2023 10:18:19 +0200 Subject: [PATCH] refactor: type safety improvements (#383) * Use curly bracket notation * fix: typos * feat: use encodeCall to make use of type checking * feature: pre-fill the OSx JIRA link so that you only need to add the number * chore: maintained changelog --- .github/pull_request_template.md | 2 +- packages/contracts/CHANGELOG.md | 1 + .../01-core/01-dao/01-actions.md | 6 +- .../01-dao/02-action-execution.md | 4 +- .../04-upgradeable-plugin/03-setup.md | 2 +- .../plugin/repo/PluginRepoFactory.sol | 2 +- .../src/framework/utils/TokenFactory.sol | 2 +- .../v1/CounterV1PluginSetup.sol | 97 +++++++------ .../v2/CounterV2PluginSetup.sol | 121 ++++++++-------- .../plugins/governance/admin/AdminSetup.sol | 42 +++--- .../addresslist/AddresslistVotingSetup.sol | 129 +++++++++--------- .../token/TokenVotingSetup.sol | 125 ++++++++--------- .../governance/multisig/MultisigSetup.sol | 92 ++++++------- .../src/plugins/token/MerkleMinter.sol | 8 +- .../src/test/plugin/PluginMockData.sol | 14 +- .../PluginUUPSUpgradeableSetupMock.sol | 12 +- 16 files changed, 324 insertions(+), 335 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 586fd82d1..a77118bff 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -2,7 +2,7 @@ Please include a summary of the change and be sure you follow the contributions rules we do provide [here](./CONTRIBUTIONS.md) -Task: [ID]() +Task ID: [OS-?](https://aragonassociation.atlassian.net/browse/OS-?) ## Type of change See the framework lifecylce in `packages/contracts/docs/framework-lifecycle` to decide what kind of change this pull request is. diff --git a/packages/contracts/CHANGELOG.md b/packages/contracts/CHANGELOG.md index c43181756..9a1ca2713 100644 --- a/packages/contracts/CHANGELOG.md +++ b/packages/contracts/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Improved type safety by using `abi.encodeCall` instead of `abi.encodeWithSelector` and the more explicit bracket syntax for permissions. - Bumped OpenZeppelin dependencies to `4.9.3`. - Refactored the fallback in the `isGranted` function in `PermissionManager` to make conditions mutually exclusive: Specific conditions answering `false` do not fall back to generic caller conditions (`_who: ANY_ADDR`) or generic target conditions (`_where: ANY_ADDR`). - Renamed the `signatureValidator` variable in `DAO` to `__removed0`. diff --git a/packages/contracts/docs/developer-portal/01-how-it-works/01-core/01-dao/01-actions.md b/packages/contracts/docs/developer-portal/01-how-it-works/01-core/01-dao/01-actions.md index 3ed729f43..b9d83de3f 100644 --- a/packages/contracts/docs/developer-portal/01-how-it-works/01-core/01-dao/01-actions.md +++ b/packages/contracts/docs/developer-portal/01-how-it-works/01-core/01-dao/01-actions.md @@ -59,9 +59,9 @@ We have an Aragon DAO deployed on the Goerli testnet. Now, we want to wrap `0.1 IDAO.Action[] memory actions = new IDAO.Action[](1); actions[0] = IDAO.Action({ - to: address(0xB4FBF271143F4FBf7B91A5ded31805e42b2208d6), // The address of the WETH contract on Goerli - value: 0.1 ether, // The Goerli ETH value to be sent with the function call - data: abi.encodeWithSelector(bytes4(keccak256('deposit()', []))) // The calldata + to: address(0xB4FBF271143F4FBf7B91A5ded31805e42b2208d6), // The address of the WETH contract on Goerli + value: 0.1 ether, // The Goerli ETH value to be sent with the function call + data: abi.encodeCall(IDAO.deposit, ()) // The calldata }); dao().execute({_callId: '', _actions: actions, _allowFailureMap: 0}); diff --git a/packages/contracts/docs/developer-portal/02-how-to-guides/01-dao/02-action-execution.md b/packages/contracts/docs/developer-portal/02-how-to-guides/01-dao/02-action-execution.md index 95bb35b8c..281f6fc5c 100644 --- a/packages/contracts/docs/developer-portal/02-how-to-guides/01-dao/02-action-execution.md +++ b/packages/contracts/docs/developer-portal/02-how-to-guides/01-dao/02-action-execution.md @@ -106,7 +106,7 @@ function exampleFunctionCall( actions[0] = IDAO.Action({ to: address(_calculator), value: 0, // 0 native tokens must be sent with this call - data: abi.encodeWithSelector(_calculator.add.selector, _a, _b) + data: abi.encodeCall(_calculator.add, (_a, _b)) }); // Execute the action array @@ -141,7 +141,7 @@ function examplePayableFunctionCall(IDAO dao, bytes32 _callId, IWETH9 _wethToken actions[0] = IDAO.Action({ to: address(_wethToken), value: 0.1 ether, - data: abi.encodeWithSelector(IWETH9.deposit.selector) // abi.encodeWithSelector(bytes4(keccak256('deposit()')), []) + data: abi.encodeCall(IWETH9.deposit, ()) }); // Execute the action array diff --git a/packages/contracts/docs/developer-portal/02-how-to-guides/02-plugin-development/04-upgradeable-plugin/03-setup.md b/packages/contracts/docs/developer-portal/02-how-to-guides/02-plugin-development/04-upgradeable-plugin/03-setup.md index 91f26fa0e..5bb4c8e84 100644 --- a/packages/contracts/docs/developer-portal/02-how-to-guides/02-plugin-development/04-upgradeable-plugin/03-setup.md +++ b/packages/contracts/docs/developer-portal/02-how-to-guides/02-plugin-development/04-upgradeable-plugin/03-setup.md @@ -76,7 +76,7 @@ contract SimpleStorageBuild1Setup is PluginSetup { plugin = createERC1967Proxy( simpleStorageImplementation, - abi.encodeWithSelector(SimpleStorageBuild1.initializeBuild1.selector, _dao, number) + abi.encodeCall(SimpleStorageBuild1.initializeBuild1, (IDAO(_dao), number)) ); PermissionLib.MultiTargetPermission[] diff --git a/packages/contracts/src/framework/plugin/repo/PluginRepoFactory.sol b/packages/contracts/src/framework/plugin/repo/PluginRepoFactory.sol index c712202be..6f4c5bb72 100644 --- a/packages/contracts/src/framework/plugin/repo/PluginRepoFactory.sol +++ b/packages/contracts/src/framework/plugin/repo/PluginRepoFactory.sol @@ -133,7 +133,7 @@ contract PluginRepoFactory is ERC165, ProtocolVersion { pluginRepo = PluginRepo( createERC1967Proxy( pluginRepoBase, - abi.encodeWithSelector(PluginRepo.initialize.selector, _initialOwner) + abi.encodeCall(PluginRepo.initialize, (_initialOwner)) ) ); diff --git a/packages/contracts/src/framework/utils/TokenFactory.sol b/packages/contracts/src/framework/utils/TokenFactory.sol index f2f0daea4..bef76233b 100644 --- a/packages/contracts/src/framework/utils/TokenFactory.sol +++ b/packages/contracts/src/framework/utils/TokenFactory.sol @@ -88,7 +88,7 @@ contract TokenFactory { if (token != address(0)) { // Validate if token is ERC20 bytes memory data = token.functionStaticCall( - abi.encodeWithSelector(IERC20Upgradeable.balanceOf.selector, address(this)) + abi.encodeCall(IERC20Upgradeable.balanceOf, (address(this))) ); if (data.length != 0x20) { diff --git a/packages/contracts/src/plugins/counter-example/v1/CounterV1PluginSetup.sol b/packages/contracts/src/plugins/counter-example/v1/CounterV1PluginSetup.sol index 7af94ab5d..1b95e75b8 100644 --- a/packages/contracts/src/plugins/counter-example/v1/CounterV1PluginSetup.sol +++ b/packages/contracts/src/plugins/counter-example/v1/CounterV1PluginSetup.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.17; import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; +import {IDAO} from "../../../core/dao/IDAO.sol"; import {PermissionLib} from "../../../core/permission/PermissionLib.sol"; import {IPluginSetup} from "../../../framework/plugin/setup/IPluginSetup.sol"; import {PluginSetup} from "../../../framework/plugin/setup/PluginSetup.sol"; @@ -44,11 +45,9 @@ contract CounterV1PluginSetup is PluginSetup { multiplyHelper = createERC1967Proxy(address(multiplyHelperBase), bytes("")); } - bytes memory initData = abi.encodeWithSelector( - bytes4(keccak256("initialize(address,address,uint256)")), - _dao, - multiplyHelper, - _num + bytes memory initData = abi.encodeCall( + CounterV1.initialize, + (IDAO(_dao), MultiplyHelper(multiplyHelper), _num) ); PermissionLib.MultiTargetPermission[] @@ -61,30 +60,30 @@ contract CounterV1PluginSetup is PluginSetup { plugin = createERC1967Proxy(address(counterBase), initData); // set permissions - permissions[0] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - _dao, - plugin, - PermissionLib.NO_CONDITION, - keccak256("EXECUTE_PERMISSION") - ); - - permissions[1] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - plugin, - _dao, - PermissionLib.NO_CONDITION, - counterBase.MULTIPLY_PERMISSION_ID() - ); + permissions[0] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: _dao, + who: plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: keccak256("EXECUTE_PERMISSION") + }); + + permissions[1] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: counterBase.MULTIPLY_PERMISSION_ID() + }); if (_multiplyHelper == address(0)) { - permissions[2] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - multiplyHelper, - plugin, - PermissionLib.NO_CONDITION, - multiplyHelperBase.MULTIPLY_PERMISSION_ID() - ); + permissions[2] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: multiplyHelper, + who: plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: multiplyHelperBase.MULTIPLY_PERMISSION_ID() + }); } // add helpers @@ -106,30 +105,30 @@ contract CounterV1PluginSetup is PluginSetup { ); // set permissions - permissions[0] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _dao, - _payload.plugin, - PermissionLib.NO_CONDITION, - keccak256("EXECUTE_PERMISSION") - ); - - permissions[1] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _payload.plugin, - _dao, - PermissionLib.NO_CONDITION, - counterBase.MULTIPLY_PERMISSION_ID() - ); + permissions[0] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _dao, + who: _payload.plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: keccak256("EXECUTE_PERMISSION") + }); + + permissions[1] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _payload.plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: counterBase.MULTIPLY_PERMISSION_ID() + }); if (_payload.currentHelpers.length != 0) { - permissions[2] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _payload.currentHelpers[0], - _payload.plugin, - PermissionLib.NO_CONDITION, - multiplyHelperBase.MULTIPLY_PERMISSION_ID() - ); + permissions[2] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _payload.currentHelpers[0], + who: _payload.plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: multiplyHelperBase.MULTIPLY_PERMISSION_ID() + }); } } diff --git a/packages/contracts/src/plugins/counter-example/v2/CounterV2PluginSetup.sol b/packages/contracts/src/plugins/counter-example/v2/CounterV2PluginSetup.sol index 890721e93..baa260832 100644 --- a/packages/contracts/src/plugins/counter-example/v2/CounterV2PluginSetup.sol +++ b/packages/contracts/src/plugins/counter-example/v2/CounterV2PluginSetup.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.17; import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; +import {IDAO} from "../../../core/dao/IDAO.sol"; import {PermissionLib} from "../../../core/permission/PermissionLib.sol"; import {IPluginSetup} from "../../../framework/plugin/setup/IPluginSetup.sol"; import {PluginSetup} from "../../../framework/plugin/setup/PluginSetup.sol"; @@ -37,7 +38,10 @@ contract CounterV2PluginSetup is PluginSetup { returns (address plugin, PreparedSetupData memory preparedSetupData) { // Decode the parameters from the UI - (address _multiplyHelper, uint256 _num) = abi.decode(_data, (address, uint256)); + (address _multiplyHelper, uint256 _num, uint256 _newVariable) = abi.decode( + _data, + (address, uint256, uint256) + ); address multiplyHelper = _multiplyHelper; @@ -45,11 +49,9 @@ contract CounterV2PluginSetup is PluginSetup { multiplyHelper = createERC1967Proxy(address(multiplyHelperBase), bytes("")); } - bytes memory initData = abi.encodeWithSelector( - bytes4(keccak256("initialize(address,address,uint256)")), - _dao, - multiplyHelper, - _num + bytes memory initData = abi.encodeCall( + CounterV2.initialize, + (IDAO(_dao), MultiplyHelper(multiplyHelper), _num, _newVariable) ); PermissionLib.MultiTargetPermission[] @@ -62,30 +64,30 @@ contract CounterV2PluginSetup is PluginSetup { plugin = createERC1967Proxy(address(counterBase), initData); // set permissions - permissions[0] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - _dao, - plugin, - PermissionLib.NO_CONDITION, - keccak256("EXECUTE_PERMISSION") - ); - - permissions[1] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - plugin, - _dao, - PermissionLib.NO_CONDITION, - counterBase.MULTIPLY_PERMISSION_ID() - ); + permissions[0] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: _dao, + who: plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: keccak256("EXECUTE_PERMISSION") + }); + + permissions[1] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: counterBase.MULTIPLY_PERMISSION_ID() + }); if (_multiplyHelper == address(0)) { - permissions[2] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - multiplyHelper, - plugin, - PermissionLib.NO_CONDITION, - multiplyHelperBase.MULTIPLY_PERMISSION_ID() - ); + permissions[2] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: multiplyHelper, + who: plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: multiplyHelperBase.MULTIPLY_PERMISSION_ID() + }); } // add helpers @@ -112,21 +114,18 @@ contract CounterV2PluginSetup is PluginSetup { if (_currentBuild == 1) { (_newVariable) = abi.decode(_payload.data, (uint256)); - initData = abi.encodeWithSelector( - bytes4(keccak256("setNewVariable(uint256)")), - _newVariable - ); + initData = abi.encodeCall(CounterV2.setNewVariable, (_newVariable)); } PermissionLib.MultiTargetPermission[] memory permissions = new PermissionLib.MultiTargetPermission[](1); - permissions[0] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _dao, - _payload.plugin, - PermissionLib.NO_CONDITION, - multiplyHelperBase.MULTIPLY_PERMISSION_ID() - ); + permissions[0] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _dao, + who: _payload.plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: multiplyHelperBase.MULTIPLY_PERMISSION_ID() + }); // if another helper is deployed, put it inside activeHelpers + put old ones as well. address[] memory activeHelpers = new address[](1); @@ -146,30 +145,30 @@ contract CounterV2PluginSetup is PluginSetup { ); // set permissions - permissions[0] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _dao, - _payload.plugin, - PermissionLib.NO_CONDITION, - keccak256("EXECUTE_PERMISSION") - ); - - permissions[1] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _payload.plugin, - _dao, - PermissionLib.NO_CONDITION, - counterBase.MULTIPLY_PERMISSION_ID() - ); + permissions[0] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _dao, + who: _payload.plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: keccak256("EXECUTE_PERMISSION") + }); + + permissions[1] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _payload.plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: counterBase.MULTIPLY_PERMISSION_ID() + }); if (_payload.currentHelpers.length != 0) { - permissions[2] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _payload.currentHelpers[0], - _payload.plugin, - PermissionLib.NO_CONDITION, - multiplyHelperBase.MULTIPLY_PERMISSION_ID() - ); + permissions[2] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _payload.currentHelpers[0], + who: _payload.plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: multiplyHelperBase.MULTIPLY_PERMISSION_ID() + }); } } diff --git a/packages/contracts/src/plugins/governance/admin/AdminSetup.sol b/packages/contracts/src/plugins/governance/admin/AdminSetup.sol index a1ab5de21..452b7ef32 100644 --- a/packages/contracts/src/plugins/governance/admin/AdminSetup.sol +++ b/packages/contracts/src/plugins/governance/admin/AdminSetup.sol @@ -52,22 +52,22 @@ contract AdminSetup is PluginSetup { memory permissions = new PermissionLib.MultiTargetPermission[](2); // Grant `ADMIN_EXECUTE_PERMISSION` of the plugin to the admin. - permissions[0] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - plugin, - admin, - PermissionLib.NO_CONDITION, - Admin(plugin).EXECUTE_PROPOSAL_PERMISSION_ID() - ); + permissions[0] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: plugin, + who: admin, + condition: PermissionLib.NO_CONDITION, + permissionId: Admin(plugin).EXECUTE_PROPOSAL_PERMISSION_ID() + }); // Grant `EXECUTE_PERMISSION` on the DAO to the plugin. - permissions[1] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - _dao, - plugin, - PermissionLib.NO_CONDITION, - DAO(payable(_dao)).EXECUTE_PERMISSION_ID() - ); + permissions[1] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: _dao, + who: plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() + }); preparedSetupData.permissions = permissions; } @@ -81,13 +81,13 @@ contract AdminSetup is PluginSetup { // Prepare permissions permissions = new PermissionLib.MultiTargetPermission[](1); - permissions[0] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _dao, - _payload.plugin, - PermissionLib.NO_CONDITION, - DAO(payable(_dao)).EXECUTE_PERMISSION_ID() - ); + permissions[0] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _dao, + who: _payload.plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() + }); } /// @inheritdoc IPluginSetup diff --git a/packages/contracts/src/plugins/governance/majority-voting/addresslist/AddresslistVotingSetup.sol b/packages/contracts/src/plugins/governance/majority-voting/addresslist/AddresslistVotingSetup.sol index 78510e181..0b3665720 100644 --- a/packages/contracts/src/plugins/governance/majority-voting/addresslist/AddresslistVotingSetup.sol +++ b/packages/contracts/src/plugins/governance/majority-voting/addresslist/AddresslistVotingSetup.sol @@ -34,12 +34,7 @@ contract AddresslistVotingSetup is PluginSetup { // Prepare and Deploy the plugin proxy. plugin = createERC1967Proxy( address(addresslistVotingBase), - abi.encodeWithSelector( - AddresslistVoting.initialize.selector, - _dao, - votingSettings, - members - ) + abi.encodeCall(AddresslistVoting.initialize, (IDAO(_dao), votingSettings, members)) ); // Prepare permissions @@ -48,38 +43,38 @@ contract AddresslistVotingSetup is PluginSetup { // Set permissions to be granted. // Grant the list of permissions of the plugin to the DAO. - permissions[0] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - plugin, - _dao, - PermissionLib.NO_CONDITION, - addresslistVotingBase.UPDATE_ADDRESSES_PERMISSION_ID() - ); - - permissions[1] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - plugin, - _dao, - PermissionLib.NO_CONDITION, - addresslistVotingBase.UPDATE_VOTING_SETTINGS_PERMISSION_ID() - ); - - permissions[2] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - plugin, - _dao, - PermissionLib.NO_CONDITION, - addresslistVotingBase.UPGRADE_PLUGIN_PERMISSION_ID() - ); + permissions[0] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: addresslistVotingBase.UPDATE_ADDRESSES_PERMISSION_ID() + }); + + permissions[1] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: addresslistVotingBase.UPDATE_VOTING_SETTINGS_PERMISSION_ID() + }); + + permissions[2] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: addresslistVotingBase.UPGRADE_PLUGIN_PERMISSION_ID() + }); // Grant `EXECUTE_PERMISSION` of the DAO to the plugin. - permissions[3] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - _dao, - plugin, - PermissionLib.NO_CONDITION, - DAO(payable(_dao)).EXECUTE_PERMISSION_ID() - ); + permissions[3] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: _dao, + who: plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() + }); preparedSetupData.permissions = permissions; } @@ -93,37 +88,37 @@ contract AddresslistVotingSetup is PluginSetup { permissions = new PermissionLib.MultiTargetPermission[](4); // Set permissions to be Revoked. - permissions[0] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _payload.plugin, - _dao, - PermissionLib.NO_CONDITION, - addresslistVotingBase.UPDATE_ADDRESSES_PERMISSION_ID() - ); - - permissions[1] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _payload.plugin, - _dao, - PermissionLib.NO_CONDITION, - addresslistVotingBase.UPDATE_VOTING_SETTINGS_PERMISSION_ID() - ); - - permissions[2] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _payload.plugin, - _dao, - PermissionLib.NO_CONDITION, - addresslistVotingBase.UPGRADE_PLUGIN_PERMISSION_ID() - ); - - permissions[3] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _dao, - _payload.plugin, - PermissionLib.NO_CONDITION, - DAO(payable(_dao)).EXECUTE_PERMISSION_ID() - ); + permissions[0] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _payload.plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: addresslistVotingBase.UPDATE_ADDRESSES_PERMISSION_ID() + }); + + permissions[1] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _payload.plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: addresslistVotingBase.UPDATE_VOTING_SETTINGS_PERMISSION_ID() + }); + + permissions[2] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _payload.plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: addresslistVotingBase.UPGRADE_PLUGIN_PERMISSION_ID() + }); + + permissions[3] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _dao, + who: _payload.plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() + }); } /// @inheritdoc IPluginSetup diff --git a/packages/contracts/src/plugins/governance/majority-voting/token/TokenVotingSetup.sol b/packages/contracts/src/plugins/governance/majority-voting/token/TokenVotingSetup.sol index 4f30c8639..d9981b240 100644 --- a/packages/contracts/src/plugins/governance/majority-voting/token/TokenVotingSetup.sol +++ b/packages/contracts/src/plugins/governance/majority-voting/token/TokenVotingSetup.sol @@ -139,7 +139,10 @@ contract TokenVotingSetup is PluginSetup { // Prepare and deploy plugin proxy. plugin = createERC1967Proxy( address(tokenVotingBase), - abi.encodeWithSelector(TokenVoting.initialize.selector, _dao, votingSettings, token) + abi.encodeCall( + TokenVoting.initialize, + (IDAO(_dao), votingSettings, IVotesUpgradeable(token)) + ) ); // Prepare permissions @@ -150,41 +153,41 @@ contract TokenVotingSetup is PluginSetup { // Set plugin permissions to be granted. // Grant the list of permissions of the plugin to the DAO. - permissions[0] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - plugin, - _dao, - PermissionLib.NO_CONDITION, - tokenVotingBase.UPDATE_VOTING_SETTINGS_PERMISSION_ID() - ); - - permissions[1] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - plugin, - _dao, - PermissionLib.NO_CONDITION, - tokenVotingBase.UPGRADE_PLUGIN_PERMISSION_ID() - ); + permissions[0] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: tokenVotingBase.UPDATE_VOTING_SETTINGS_PERMISSION_ID() + }); + + permissions[1] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: tokenVotingBase.UPGRADE_PLUGIN_PERMISSION_ID() + }); // Grant `EXECUTE_PERMISSION` of the DAO to the plugin. - permissions[2] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - _dao, - plugin, - PermissionLib.NO_CONDITION, - DAO(payable(_dao)).EXECUTE_PERMISSION_ID() - ); + permissions[2] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: _dao, + who: plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() + }); if (tokenSettings.addr == address(0)) { bytes32 tokenMintPermission = GovernanceERC20(token).MINT_PERMISSION_ID(); - permissions[3] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - token, - _dao, - PermissionLib.NO_CONDITION, - tokenMintPermission - ); + permissions[3] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: token, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: tokenMintPermission + }); } preparedSetupData.helpers = helpers; @@ -213,41 +216,41 @@ contract TokenVotingSetup is PluginSetup { permissions = new PermissionLib.MultiTargetPermission[](isGovernanceERC20 ? 4 : 3); // Set permissions to be Revoked. - permissions[0] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _payload.plugin, - _dao, - PermissionLib.NO_CONDITION, - tokenVotingBase.UPDATE_VOTING_SETTINGS_PERMISSION_ID() - ); - - permissions[1] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _payload.plugin, - _dao, - PermissionLib.NO_CONDITION, - tokenVotingBase.UPGRADE_PLUGIN_PERMISSION_ID() - ); - - permissions[2] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _dao, - _payload.plugin, - PermissionLib.NO_CONDITION, - DAO(payable(_dao)).EXECUTE_PERMISSION_ID() - ); + permissions[0] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _payload.plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: tokenVotingBase.UPDATE_VOTING_SETTINGS_PERMISSION_ID() + }); + + permissions[1] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _payload.plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: tokenVotingBase.UPGRADE_PLUGIN_PERMISSION_ID() + }); + + permissions[2] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _dao, + who: _payload.plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() + }); // Revocation of permission is necessary only if the deployed token is GovernanceERC20, // as GovernanceWrapped does not possess this permission. Only return the following // if it's type of GovernanceERC20, otherwise revoking this permission wouldn't have any effect. if (isGovernanceERC20) { - permissions[3] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - token, - _dao, - PermissionLib.NO_CONDITION, - GovernanceERC20(token).MINT_PERMISSION_ID() - ); + permissions[3] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: token, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: GovernanceERC20(token).MINT_PERMISSION_ID() + }); } } @@ -272,7 +275,7 @@ contract TokenVotingSetup is PluginSetup { /// @param token The token address function _isERC20(address token) private view returns (bool) { (bool success, bytes memory data) = token.staticcall( - abi.encodeWithSelector(IERC20Upgradeable.balanceOf.selector, address(this)) + abi.encodeCall(IERC20Upgradeable.balanceOf, (address(this))) ); return success && data.length == 0x20; } diff --git a/packages/contracts/src/plugins/governance/multisig/MultisigSetup.sol b/packages/contracts/src/plugins/governance/multisig/MultisigSetup.sol index 732bfc225..6a334d6a6 100644 --- a/packages/contracts/src/plugins/governance/multisig/MultisigSetup.sol +++ b/packages/contracts/src/plugins/governance/multisig/MultisigSetup.sol @@ -35,7 +35,7 @@ contract MultisigSetup is PluginSetup { // Prepare and Deploy the plugin proxy. plugin = createERC1967Proxy( address(multisigBase), - abi.encodeWithSelector(Multisig.initialize.selector, _dao, members, multisigSettings) + abi.encodeCall(Multisig.initialize, (IDAO(_dao), members, multisigSettings)) ); // Prepare permissions @@ -44,30 +44,30 @@ contract MultisigSetup is PluginSetup { // Set permissions to be granted. // Grant the list of permissions of the plugin to the DAO. - permissions[0] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - plugin, - _dao, - PermissionLib.NO_CONDITION, - multisigBase.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() - ); - - permissions[1] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - plugin, - _dao, - PermissionLib.NO_CONDITION, - multisigBase.UPGRADE_PLUGIN_PERMISSION_ID() - ); + permissions[0] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: multisigBase.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() + }); + + permissions[1] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: multisigBase.UPGRADE_PLUGIN_PERMISSION_ID() + }); // Grant `EXECUTE_PERMISSION` of the DAO to the plugin. - permissions[2] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Grant, - _dao, - plugin, - PermissionLib.NO_CONDITION, - DAO(payable(_dao)).EXECUTE_PERMISSION_ID() - ); + permissions[2] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: _dao, + who: plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() + }); preparedSetupData.permissions = permissions; } @@ -93,29 +93,29 @@ contract MultisigSetup is PluginSetup { permissions = new PermissionLib.MultiTargetPermission[](3); // Set permissions to be Revoked. - permissions[0] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _payload.plugin, - _dao, - PermissionLib.NO_CONDITION, - multisigBase.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() - ); - - permissions[1] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _payload.plugin, - _dao, - PermissionLib.NO_CONDITION, - multisigBase.UPGRADE_PLUGIN_PERMISSION_ID() - ); - - permissions[2] = PermissionLib.MultiTargetPermission( - PermissionLib.Operation.Revoke, - _dao, - _payload.plugin, - PermissionLib.NO_CONDITION, - DAO(payable(_dao)).EXECUTE_PERMISSION_ID() - ); + permissions[0] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _payload.plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: multisigBase.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() + }); + + permissions[1] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _payload.plugin, + who: _dao, + condition: PermissionLib.NO_CONDITION, + permissionId: multisigBase.UPGRADE_PLUGIN_PERMISSION_ID() + }); + + permissions[2] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _dao, + who: _payload.plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() + }); } /// @inheritdoc IPluginSetup diff --git a/packages/contracts/src/plugins/token/MerkleMinter.sol b/packages/contracts/src/plugins/token/MerkleMinter.sol index ef8909254..95fb5587a 100644 --- a/packages/contracts/src/plugins/token/MerkleMinter.sol +++ b/packages/contracts/src/plugins/token/MerkleMinter.sol @@ -80,11 +80,9 @@ contract MerkleMinter is IMerkleMinter, PluginUUPSUpgradeable { ) external override auth(MERKLE_MINT_PERMISSION_ID) returns (IMerkleDistributor distributor) { address distributorAddr = createERC1967Proxy( address(distributorBase), - abi.encodeWithSelector( - MerkleDistributor.initialize.selector, - dao(), - IERC20Upgradeable(address(token)), - _merkleRoot + abi.encodeCall( + MerkleDistributor.initialize, + (dao(), IERC20Upgradeable(address(token)), _merkleRoot) ) ); diff --git a/packages/contracts/src/test/plugin/PluginMockData.sol b/packages/contracts/src/test/plugin/PluginMockData.sol index 9c12a4232..0bc2b8089 100644 --- a/packages/contracts/src/test/plugin/PluginMockData.sol +++ b/packages/contracts/src/test/plugin/PluginMockData.sol @@ -16,13 +16,13 @@ function mockPermissions( permissions = new PermissionLib.MultiTargetPermission[](end - start); for (uint160 i = start; i < end; i++) { - permissions[i - start] = PermissionLib.MultiTargetPermission( - op, - address(i), - address(i), - PermissionLib.NO_CONDITION, - keccak256("MOCK_PERMISSION") - ); + permissions[i - start] = PermissionLib.MultiTargetPermission({ + operation: op, + where: address(i), + who: address(i), + condition: PermissionLib.NO_CONDITION, + permissionId: keccak256("MOCK_PERMISSION") + }); } } diff --git a/packages/contracts/src/test/plugin/UUPSUpgradeable/PluginUUPSUpgradeableSetupMock.sol b/packages/contracts/src/test/plugin/UUPSUpgradeable/PluginUUPSUpgradeableSetupMock.sol index 482d5b94a..2c8330344 100644 --- a/packages/contracts/src/test/plugin/UUPSUpgradeable/PluginUUPSUpgradeableSetupMock.sol +++ b/packages/contracts/src/test/plugin/UUPSUpgradeable/PluginUUPSUpgradeableSetupMock.sol @@ -83,9 +83,7 @@ contract PluginUUPSUpgradeableSetupV2Mock is PluginUUPSUpgradeableSetupV1Mock { // Update from V1 if (_currentBuild == 1) { preparedSetupData.helpers = mockHelpers(2); - initData = abi.encodeWithSelector( - PluginUUPSUpgradeableV2Mock.initializeV1toV2.selector - ); + initData = abi.encodeCall(PluginUUPSUpgradeableV2Mock.initializeV1toV2, ()); preparedSetupData.permissions = mockPermissions(1, 2, PermissionLib.Operation.Grant); } } @@ -121,18 +119,14 @@ contract PluginUUPSUpgradeableSetupV3Mock is PluginUUPSUpgradeableSetupV2Mock { // Update from V1 if (_currentBuild == 1) { preparedSetupData.helpers = mockHelpers(3); - initData = abi.encodeWithSelector( - PluginUUPSUpgradeableV3Mock.initializeV1toV3.selector - ); + initData = abi.encodeCall(PluginUUPSUpgradeableV3Mock.initializeV1toV3, ()); preparedSetupData.permissions = mockPermissions(1, 3, PermissionLib.Operation.Grant); } // Update from V2 if (_currentBuild == 2) { preparedSetupData.helpers = mockHelpers(3); - initData = abi.encodeWithSelector( - PluginUUPSUpgradeableV3Mock.initializeV2toV3.selector - ); + initData = abi.encodeCall(PluginUUPSUpgradeableV3Mock.initializeV2toV3, ()); preparedSetupData.permissions = mockPermissions(2, 3, PermissionLib.Operation.Grant); } }