refactor: DAO signature validation

packages/contracts-ethers/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@aragon/osx-ethers",
-  "version": "1.3.1-rc0",
+  "version": "1.4.0-rc0",
   "description": "The Aragon OSx contract definitions for ethers.js",
   "main": "dist/bundle-cjs.js",
   "module": "dist/bundle-esm.js",

packages/contracts/CHANGELOG.md

@@ -5,6 +5,23 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## v1.4.0-rc0
+
+### Added
+
+- Added the `FunctionRemoved` error to `DAO`.
+
+### Changed
+
+- Renamed the `signatureValidator` variable in `DAO` to `__removed0`.
+- Use the DAOs permission manager functionality to validate signatures.
+
+### Removed
+
+- Removed the `SignatureValidatorSet` event from `IDAO`.
+- Removed unused `ERC1271Mock` contract.
+- Removed the `setSignatureValidator` function and `signatureValidator` variable in `DAO`. In places, where the function must remain to not alter the `IDAO` interface ID, it will revert and explanatory notes are put in place..
+
 ## v1.3.1-rc0
 
 ### Added

packages/contracts/docs/osx/01-how-it-works/01-core/01-dao/index.md

@@ -50,7 +50,7 @@ Currently, externally owned accounts (EOAs) can sign messages with their associa
 An exemplary use case is a decentralized exchange with an off-chain order book, where buy/sell orders are signed messages.
 To accept such a request, both, the external service provider and caller need to follow a standard with which the signed message of the caller can be validated.
 
-By supporting the [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) standard, your DAO can validate signatures via its `isValidSignature` function that forwards the call to a signature validator contract. The signature validator can be set with `setSignatureValidator` function.
+By supporting the [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) standard, your DAO can validate signatures via its `isValidSignature` function that forwards the call to a signature validator contract.
 
 <!-- Add a subsection explaining how signature validation works -->
 

packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md

@@ -145,7 +145,6 @@ The following functions in the DAO are permissioned:
 | `_authorizeUpgrade`                     | `UPGRADE_DAO_PERMISSION_ID`                | Required to upgrade the DAO (via the [UUPS](https://eips.ethereum.org/EIPS/eip-1822)).                          |
 | `setMetadata`                           | `SET_METADATA_PERMISSION_ID`               | Required to set the DAO’s metadata and [DAOstar.one DAO URI](https://eips.ethereum.org/EIPS/eip-4824).          |
 | `setTrustedForwarder`                   | `SET_TRUSTED_FORWARDER_PERMISSION_ID`      | Required to set the DAO’s trusted forwarder for meta transactions.                                              |
-| `setSignatureValidator`                 | `SET_SIGNATURE_VALIDATOR_PERMISSION_ID`    | Required to set the DAO’s signature validator contract (see ERC-1271).                                          |
 | `registerStandardCallback`              | `REGISTER_STANDARD_CALLBACK_PERMISSION_ID` | Required to register a standard callback for an [ERC-165](https://eips.ethereum.org/EIPS/eip-165) interface ID. |
 
 Plugins installed on the DAO might introduce other permissions and associated permission identifiers.

packages/contracts/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@aragon/osx-artifacts",
-  "version": "1.3.1-rc0",
+  "version": "1.4.0-rc0",
   "description": "The Aragon OSx Solidity contracts",
   "main": "dist/bundle-cjs.js",
   "module": "dist/bundle-esm.js",
@@ -42,6 +42,7 @@
   "devDependencies": {
     "@aragon/osx-ethers-v1.2.0": "npm:@aragon/[email protected]",
     "@aragon/osx-v1.0.1": "npm:@aragon/[email protected]",
+    "@aragon/osx-v1.3.0-rc0.2": "npm:@aragon/[email protected]",
     "@defi-wonderland/smock": "^2.3.4",
     "@ensdomains/ens-contracts": "0.0.11",
     "@nomicfoundation/hardhat-chai-matchers": "^1.0.5",

packages/contracts/scripts/osx-versions-aliases.ts

@@ -5,4 +5,7 @@
  *
  * To add a new version alias, append it to this array.
  */
-export const OSX_VERSION_ALIASES = ['@aragon/osx-v1.0.1/'];
+export const OSX_VERSION_ALIASES = [
+  '@aragon/osx-v1.0.1/',
+  '@aragon/osx-v1.3.0-rc0.2/',
+];

packages/contracts/src/core/dao/DAO.sol

@@ -52,14 +52,14 @@ contract DAO is
     bytes32 public constant SET_TRUSTED_FORWARDER_PERMISSION_ID =
         keccak256("SET_TRUSTED_FORWARDER_PERMISSION");
 
-    /// @notice The ID of the permission required to call the `setSignatureValidator` function.
-    bytes32 public constant SET_SIGNATURE_VALIDATOR_PERMISSION_ID =
-        keccak256("SET_SIGNATURE_VALIDATOR_PERMISSION");
-
     /// @notice The ID of the permission required to call the `registerStandardCallback` function.
     bytes32 public constant REGISTER_STANDARD_CALLBACK_PERMISSION_ID =
         keccak256("REGISTER_STANDARD_CALLBACK_PERMISSION");
 
+    /// @notice The ID of the permission required to validate [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) signatures.
+    bytes32 public constant VALIDATE_SIGNATURE_PERMISSION_ID =
+        keccak256("VALIDATE_SIGNATURE_PERMISSION");
+
     /// @notice The internal constant storing the maximal action array length.
     uint256 internal constant MAX_ACTIONS = 256;
 
@@ -69,9 +69,10 @@ contract DAO is
     /// @notice The second out of two values to which the `_reentrancyStatus` state variable (used by the `nonReentrant` modifier) can be set inidicating that a function was entered.
     uint256 private constant _ENTERED = 2;
 
-    /// @notice The [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) signature validator contract.
-    /// @dev Added in v1.0.0.
-    IERC1271 public signatureValidator;
+    /// @notice Removed variable that is left here to maintain the storage layout.
+    /// @dev Introducedd in v1.0.0. Removed in v1.4.0.
+    /// @custom:oz-renamed-from signatureValidator
+    address private __removed1;
 
     /// @notice The address of the trusted forwarder verifying meta transactions.
     /// @dev Added in v1.0.0.
@@ -109,6 +110,9 @@ contract DAO is
     /// @notice Thrown if an upgrade is not supported from a specific protocol version .
     error ProtocolVersionUpgradeNotSupported(uint8[3] protocolVersion);
 
+    /// @notice Thrown when a function is removed but left to not corrupt the interface ID.
+    error FunctionRemoved();
+
     /// @notice Emitted when a new DAO URI is set.
     /// @param daoURI The new URI.
     event NewURI(string daoURI);
@@ -193,7 +197,6 @@ contract DAO is
             _permissionId == UPGRADE_DAO_PERMISSION_ID ||
             _permissionId == SET_METADATA_PERMISSION_ID ||
             _permissionId == SET_TRUSTED_FORWARDER_PERMISSION_ID ||
-            _permissionId == SET_SIGNATURE_VALIDATOR_PERMISSION_ID ||
             _permissionId == REGISTER_STANDARD_CALLBACK_PERMISSION_ID;
     }
 
@@ -320,25 +323,30 @@ contract DAO is
     }
 
     /// @inheritdoc IDAO
-    function setSignatureValidator(
-        address _signatureValidator
-    ) external override auth(SET_SIGNATURE_VALIDATOR_PERMISSION_ID) {
-        signatureValidator = IERC1271(_signatureValidator);
-
-        emit SignatureValidatorSet({signatureValidator: _signatureValidator});
+    function setSignatureValidator(address) external pure override {
+        revert FunctionRemoved();
     }
 
     /// @inheritdoc IDAO
+    /// @dev Relays the validation logic determining who is allowed to sign on behalf of the DAO to its permission manager.
+    /// Caller specific bypassing can be set direct granting (i.e., `grant({_where: dao, _who: specificErc1271Caller, _permissionId: VALIDATE_SIGNATURE_PERMISSION_ID})`).
+    /// Caller specific signature validation logic can be set by granting with a `PermissionCondition` (i.e., `grantWithCondition({_where: dao, _who: specificErc1271Caller, _permissionId: VALIDATE_SIGNATURE_PERMISSION_ID, _condition: yourConditionImplementation})`)
+    /// Generic signature validation logic can be set for all calling contracts by granting with a `PermissionCondition` to `PermissionManager.ANY_ADDR()` (i.e., `grantWithCondition({_where: dao, _who: PermissionManager.ANY_ADDR(), _permissionId: VALIDATE_SIGNATURE_PERMISSION_ID, _condition: yourConditionImplementation})`).
     function isValidSignature(
         bytes32 _hash,
         bytes memory _signature
     ) external view override(IDAO, IERC1271) returns (bytes4) {
-        if (address(signatureValidator) == address(0)) {
-            // Return the invalid magic number
-            return bytes4(0);
+        if (
+            isGranted({
+                _where: address(this),
+                _who: msg.sender,
+                _permissionId: VALIDATE_SIGNATURE_PERMISSION_ID,
+                _data: abi.encode(_hash, _signature)
+            })
+        ) {
+            return 0x1626ba7e; // `type(IERC1271).interfaceId` = bytes4(keccak256("isValidSignature(bytes32,bytes)")`
         }
-        // Forward the call to the set signature validator contract
-        return signatureValidator.isValidSignature(_hash, _signature);
+        return 0xffffffff; // `bytes4(uint32(type(uint32).max-1))`
     }
 
     /// @notice Emits the `NativeTokenDeposited` event to track native token deposits that weren't made via the deposit method.

packages/contracts/src/core/dao/IDAO.sol

@@ -112,15 +112,7 @@ interface IDAO {
     /// @param forwarder the new forwarder address.
     event TrustedForwarderSet(address forwarder);
 
-    /// @notice Setter for the [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) signature validator contract.
-    /// @param _signatureValidator The address of the signature validator.
-    function setSignatureValidator(address _signatureValidator) external;
-
-    /// @notice Emitted when the signature validator address is updated.
-    /// @param signatureValidator The address of the signature validator.
-    event SignatureValidatorSet(address signatureValidator);
-
-    /// @notice Checks whether a signature is valid for the provided hash by forwarding the call to the set [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) signature validator contract.
+    /// @notice Checks whether a signature is valid for a provided hash according to [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271).
     /// @param _hash The hash of the data to be signed.
     /// @param _signature The signature byte array associated with `_hash`.
     /// @return Returns the `bytes4` magic value `0x1626ba7e` if the signature is valid.
@@ -135,4 +127,8 @@ interface IDAO {
         bytes4 _callbackSelector,
         bytes4 _magicNumber
     ) external;
+
+    /// @notice Removed function being left here to not corrupt the IDAO interface ID. Any call will revert.
+    /// @dev Introduced in v1.0.0. Removed in v1.4.0.
+    function setSignatureValidator(address) external;
 }

packages/contracts/src/core/permission/PermissionManager.sol

@@ -60,7 +60,6 @@ abstract contract PermissionManager is Initializable {
     error ConditionInterfacNotSupported(IPermissionCondition condition);
 
     /// @notice Thrown for `ROOT_PERMISSION_ID` or `EXECUTE_PERMISSION_ID` permission grants where `who` or `where` is `ANY_ADDR`.
-
     error PermissionsForAnyAddressDisallowed();
 
     /// @notice Thrown for permission grants where `who` and `where` are both `ANY_ADDR`.

packages/contracts/src/framework/dao/DAOFactory.sol

@@ -169,7 +169,7 @@ contract DAOFactory is ERC165, ProtocolVersion {
     function _setDAOPermissions(DAO _dao) internal {
         // set permissionIds on the dao itself.
         PermissionLib.SingleTargetPermission[]
-            memory items = new PermissionLib.SingleTargetPermission[](6);
+            memory items = new PermissionLib.SingleTargetPermission[](5);
 
         // Grant DAO all the permissions required
         items[0] = PermissionLib.SingleTargetPermission(
@@ -183,21 +183,16 @@ contract DAOFactory is ERC165, ProtocolVersion {
             _dao.UPGRADE_DAO_PERMISSION_ID()
         );
         items[2] = PermissionLib.SingleTargetPermission(
-            PermissionLib.Operation.Grant,
-            address(_dao),
-            _dao.SET_SIGNATURE_VALIDATOR_PERMISSION_ID()
-        );
-        items[3] = PermissionLib.SingleTargetPermission(
             PermissionLib.Operation.Grant,
             address(_dao),
             _dao.SET_TRUSTED_FORWARDER_PERMISSION_ID()
         );
-        items[4] = PermissionLib.SingleTargetPermission(
+        items[3] = PermissionLib.SingleTargetPermission(
             PermissionLib.Operation.Grant,
             address(_dao),
             _dao.SET_METADATA_PERMISSION_ID()
         );
-        items[5] = PermissionLib.SingleTargetPermission(
+        items[4] = PermissionLib.SingleTargetPermission(
             PermissionLib.Operation.Grant,
             address(_dao),
             _dao.REGISTER_STANDARD_CALLBACK_PERMISSION_ID()

packages/contracts/src/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@aragon/osx",
-  "version": "1.3.1-rc0",
+  "version": "1.4.0-rc0",
   "description": "The Aragon OSx Solidity contracts",
   "publishConfig": {
     "access": "public"

packages/contracts/src/test/Migration.sol

@@ -20,14 +20,30 @@ pragma solidity 0.8.17;
  */
 
 import {DAO as DAO_v1_0_0} from "@aragon/osx-v1.0.1/core/dao/DAO.sol";
+import {DAO as DAO_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/core/dao/DAO.sol";
 import {DAORegistry as DAORegistry_v1_0_0} from "@aragon/osx-v1.0.1/framework/dao/DAORegistry.sol";
+import {DAORegistry as DAORegistry_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/framework/dao/DAORegistry.sol";
+
 import {PluginRepo as PluginRepo_v1_0_0} from "@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepo.sol";
+import {PluginRepo as PluginRepo_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/framework/plugin/repo/PluginRepo.sol";
+
 import {PluginRepoRegistry as PluginRepoRegistry_v1_0_0} from "@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepoRegistry.sol";
+import {PluginRepoRegistry as PluginRepoRegistry_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/framework/plugin/repo/PluginRepoRegistry.sol";
+
 import {ENSSubdomainRegistrar as ENSSubdomainRegistrar_v1_0_0} from "@aragon/osx-v1.0.1/framework/utils/ens/ENSSubdomainRegistrar.sol";
+import {ENSSubdomainRegistrar as ENSSubdomainRegistrar_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/framework/utils/ens/ENSSubdomainRegistrar.sol";
 
 import {TokenVoting as TokenVoting_v1_0_0} from "@aragon/osx-v1.0.1/plugins/governance/majority-voting/token/TokenVoting.sol";
+import {TokenVoting as TokenVoting_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/plugins/governance/majority-voting/token/TokenVoting.sol";
+
 import {AddresslistVoting as AddresslistVoting_v1_0_0} from "@aragon/osx-v1.0.1/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol";
+import {AddresslistVoting as AddresslistVoting_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol";
+
 import {Multisig as Multisig_v1_0_0} from "@aragon/osx-v1.0.1/plugins/governance/multisig/Multisig.sol";
+import {Multisig as Multisig_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/plugins/governance/multisig/Multisig.sol";
 
 import {MerkleMinter as MerkleMinter_v1_0_0} from "@aragon/osx-v1.0.1/plugins/token/MerkleMinter.sol";
+import {MerkleMinter as MerkleMinter_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/plugins/token/MerkleMinter.sol";
+
 import {MerkleDistributor as MerkleDistributor_v1_0_0} from "@aragon/osx-v1.0.1/plugins/token/MerkleDistributor.sol";
+import {MerkleDistributor as MerkleDistributor_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/plugins/token/MerkleDistributor.sol";

packages/contracts/src/test/permission/PermissionConditionMock.sol

@@ -5,18 +5,23 @@ pragma solidity 0.8.17;
 import "../../core/permission/PermissionCondition.sol";
 
 contract PermissionConditionMock is PermissionCondition {
-    bool internal _hasPermissionsResult = true;
+    bool public answer;
 
-    function isGranted(
-        address /* _where */,
-        address /* _who */,
-        bytes32 /* _permissionId */,
-        bytes memory /* _data */
-    ) external view returns (bool) {
-        return _hasPermissionsResult;
+    constructor() {
+        answer = true;
+    }
+
+    function setAnswer(bool _answer) external {
+        answer = _answer;
     }
 
-    function setWillPerform(bool _result) external {
-        _hasPermissionsResult = _result;
+    function isGranted(
+        address _where,
+        address _who,
+        bytes32 _permissionId,
+        bytes memory _data
+    ) external view returns (bool) {
+        (_where, _who, _permissionId, _data);
+        return answer;
     }
 }

packages/contracts/src/utils/protocol/ProtocolVersion.sol

@@ -13,6 +13,6 @@ abstract contract ProtocolVersion is IProtocolVersion {
 
     /// @inheritdoc IProtocolVersion
     function protocolVersion() public pure returns (uint8[3] memory) {
-        return [1, 3, 0];
+        return [1, 4, 0];
     }
 }