From ea5dcf9602d3d7ccfd58ca79ae865d8b8c9f3645 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Wed, 23 Aug 2023 17:51:07 +0200
Subject: [PATCH 01/38] refactor: dao signature validation

---
 .../01-core/02-permissions/index.md           |   1 -
 packages/contracts/src/core/dao/DAO.sol       |  39 ++--
 packages/contracts/src/core/dao/IDAO.sol      |  12 +-
 .../src/core/permission/PermissionManager.sol |   1 -
 .../src/framework/dao/DAOFactory.sol          |  11 +-
 packages/contracts/test/core/dao/dao.ts       | 207 ++++++++++++------
 .../core/permission/permission-manager.ts     |   6 +-
 .../test/framework/dao/dao-factory.ts         |   8 -
 8 files changed, 175 insertions(+), 110 deletions(-)

diff --git a/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md b/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md
index 025328536..2090809fc 100644
--- a/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md
+++ b/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.
diff --git a/packages/contracts/src/core/dao/DAO.sol b/packages/contracts/src/core/dao/DAO.sol
index f7213f480..f23a7d04e 100644
--- a/packages/contracts/src/core/dao/DAO.sol
+++ b/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;
 
@@ -109,6 +109,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 deprecated.
+    error FunctionDeprecated();
+
     /// @notice Emitted when a new DAO URI is set.
     /// @param daoURI The new URI.
     event NewURI(string daoURI);
@@ -192,7 +195,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;
     }
 
@@ -319,25 +321,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 FunctionDeprecated();
     }
 
     /// @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.
diff --git a/packages/contracts/src/core/dao/IDAO.sol b/packages/contracts/src/core/dao/IDAO.sol
index e50fad31e..72de15094 100644
--- a/packages/contracts/src/core/dao/IDAO.sol
+++ b/packages/contracts/src/core/dao/IDAO.sol
@@ -112,15 +112,13 @@ 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 Deprecated.
+    function setSignatureValidator(address) external;
 
-    /// @notice Emitted when the signature validator address is updated.
-    /// @param signatureValidator The address of the signature validator.
-    event SignatureValidatorSet(address signatureValidator);
+    /// @notice Deprecated.
+    event SignatureValidatorSet(address);
 
-    /// @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.
diff --git a/packages/contracts/src/core/permission/PermissionManager.sol b/packages/contracts/src/core/permission/PermissionManager.sol
index 1a05a3124..57236fd61 100644
--- a/packages/contracts/src/core/permission/PermissionManager.sol
+++ b/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`.
diff --git a/packages/contracts/src/framework/dao/DAOFactory.sol b/packages/contracts/src/framework/dao/DAOFactory.sol
index d9d6a5eb2..a82a8635b 100644
--- a/packages/contracts/src/framework/dao/DAOFactory.sol
+++ b/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()
diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index 03009b115..40165752e 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -11,7 +11,6 @@ import {
   TestERC721__factory,
   TestERC1155,
   TestERC1155__factory,
-  ERC1271Mock__factory,
   GasConsumer__factory,
   DAO__factory,
   IDAO__factory,
@@ -21,6 +20,7 @@ import {
   IERC1271__factory,
   IEIP4824__factory,
   IProtocolVersion__factory,
+  PermissionConditionMock__factory,
 } from '../../../typechain';
 import {DAO__factory as DAO_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/core/dao/DAO.sol';
 
@@ -48,6 +48,7 @@ import {UPGRADE_PERMISSIONS} from '../../test-utils/permissions';
 import {ZERO_BYTES32, daoExampleURI} from '../../test-utils/dao';
 import {ExecutedEvent} from '../../../typechain/DAO';
 import {CURRENT_PROTOCOL_VERSION} from '../../test-utils/protocol-version';
+import {ANY_ADDR} from '../permission/permission-manager';
 
 chai.use(smock.matchers);
 
@@ -82,8 +83,8 @@ export const PERMISSION_IDS = {
   UPGRADE_DAO_PERMISSION_ID: UPGRADE_PERMISSIONS.UPGRADE_DAO_PERMISSION_ID,
   SET_METADATA_PERMISSION_ID: ethers.utils.id('SET_METADATA_PERMISSION'),
   EXECUTE_PERMISSION_ID: ethers.utils.id('EXECUTE_PERMISSION'),
-  SET_SIGNATURE_VALIDATOR_PERMISSION_ID: ethers.utils.id(
-    'SET_SIGNATURE_VALIDATOR_PERMISSION'
+  VALIDATE_SIGNATURE_PERMISSION_ID: ethers.utils.id(
+    'VALIDATE_SIGNATURE_PERMISSION'
   ),
   SET_TRUSTED_FORWARDER_PERMISSION_ID: ethers.utils.id(
     'SET_TRUSTED_FORWARDER_PERMISSION'
@@ -94,6 +95,9 @@ export const PERMISSION_IDS = {
   ),
 };
 
+export const VALID_ERC1271_SIGNATURE = '0x1626ba7e';
+export const INVALID_ERC1271_SIGNATURE = '0xffffffff';
+
 describe('DAO', function () {
   let signers: SignerWithAddress[];
   let ownerAddress: string;
@@ -135,7 +139,7 @@ describe('DAO', function () {
       dao.grant(
         dao.address,
         ownerAddress,
-        PERMISSION_IDS.SET_SIGNATURE_VALIDATOR_PERMISSION_ID
+        PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID
       ),
       dao.grant(
         dao.address,
@@ -1178,90 +1182,161 @@ describe('DAO', function () {
       ).to.be.eq(true);
     });
 
-    it('should return 0 if no validator is set', async () => {
+    it('allows caller specific signature validation', async () => {
+      const caller = signers[0];
+      const signer = signers[1];
+
+      const message = 'Hi!';
+      const hash = ethers.utils.hashMessage(message);
+      const goodSignature = await signer.signMessage(message);
+      const badSignature = await signer.signMessage('Bye!');
+      expect(badSignature).to.not.equal(goodSignature);
+
+      // Try to call with caller but caller has no permission
+      expect(await dao.connect(caller).isValidSignature(hash, goodSignature))
+        .to.be.revertedWithCustomError(dao, 'Unauthorized')
+        .withArgs();
+
+      // Grant the permission to validate signatures to the caller unconditionally
+      await dao.grant(
+        dao.address,
+        caller.address,
+        PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID
+      );
+
+      // The caller can validate signatures now.
+      expect(await dao.connect(caller).isValidSignature(hash, goodSignature)).to
+        .not.be.reverted;
+
+      // Because the caller is allowed unconditionally, the validation is always true.
+      expect(
+        await dao.connect(caller).isValidSignature(hash, goodSignature)
+      ).to.equal(VALID_ERC1271_SIGNATURE);
       expect(
-        await dao.isValidSignature(ethers.utils.keccak256('0x00'), '0x00')
-      ).to.be.eq('0x00000000');
+        await dao.connect(caller).isValidSignature(hash, badSignature)
+      ).to.equal(VALID_ERC1271_SIGNATURE);
     });
 
-    it('should allow only `SET_SIGNATURE_VALIDATOR_PERMISSION_ID` to set validator', async () => {
-      await expect(
-        dao
-          .connect(signers[2])
-          .setSignatureValidator(ethers.Wallet.createRandom().address)
-      )
+    it('allows caller specific signature validation', async () => {
+      const caller = signers[0];
+      const signer = signers[1];
+
+      const message = 'Hi!';
+      const hash = ethers.utils.hashMessage(message);
+      const goodSignature = await signer.signMessage(message);
+      const badSignature = await signer.signMessage('Bye!');
+      expect(badSignature).to.not.equal(goodSignature);
+
+      // Try to call with caller but caller has no permission
+      expect(await dao.connect(caller).isValidSignature(hash, goodSignature))
         .to.be.revertedWithCustomError(dao, 'Unauthorized')
-        .withArgs(
-          dao.address,
-          signers[2].address,
-          PERMISSION_IDS.SET_SIGNATURE_VALIDATOR_PERMISSION_ID
-        );
-    });
+        .withArgs();
 
-    it('should set validator and emits event', async () => {
-      const validatorAddress = ethers.Wallet.createRandom().address;
-      const tx = await dao.setSignatureValidator(validatorAddress);
+      // Grant the permission to validate signatures to the caller unconditionally
+      await dao.grant(
+        dao.address,
+        caller.address,
+        PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID
+      );
 
-      expect(await dao.signatureValidator()).to.be.eq(validatorAddress);
+      // The caller can validate signatures now.
+      expect(await dao.connect(caller).isValidSignature(hash, goodSignature)).to
+        .not.be.reverted;
 
-      await expect(tx)
-        .to.emit(dao, EVENTS.SignatureValidatorSet)
-        .withArgs(validatorAddress);
+      // Because the caller is allowed unconditionally, the signature is always valid.
+      expect(
+        await dao.connect(caller).isValidSignature(hash, goodSignature)
+      ).to.equal(VALID_ERC1271_SIGNATURE);
+      expect(
+        await dao.connect(caller).isValidSignature(hash, badSignature)
+      ).to.equal(VALID_ERC1271_SIGNATURE);
     });
 
-    it('should call the signature validator', async () => {
-      const ERC1271MockFactory = await smock.mock('ERC1271Mock');
-      const erc1271Mock = await ERC1271MockFactory.deploy();
+    it('allows generic signature validation', async () => {
+      const caller = signers[0];
+      const signer = signers[1];
+      const other = signers[3];
 
-      await dao.setSignatureValidator(erc1271Mock.address);
-      await dao.isValidSignature(ethers.utils.keccak256('0x00'), '0x00');
-      expect(erc1271Mock.isValidSignature).has.been.callCount(1);
-    });
+      const message = 'Hi!';
+      const hash = ethers.utils.hashMessage(message);
+      const goodSignature = await signer.signMessage(message);
+      const badSignature = await signer.signMessage('Bye!');
+      expect(badSignature).to.not.equal(goodSignature);
+
+      // Try to call with caller but caller has no permission
+      expect(await dao.connect(caller).isValidSignature(hash, goodSignature))
+        .to.be.revertedWithCustomError(dao, 'Unauthorized')
+        .withArgs();
 
-    it('should return the validators response', async () => {
-      const ERC1271MockFactory = new ERC1271Mock__factory(signers[0]);
-      const erc1271Mock = await ERC1271MockFactory.deploy();
+      const mockCondition = await new PermissionConditionMock__factory(
+        caller
+      ).deploy();
 
-      await dao.setSignatureValidator(erc1271Mock.address);
+      // Grant the permission to validate signatures to the caller conditionally (granting it unconditionally is not possible in combination with `_who: ANY_ADDR`)
+      await dao.grantWithCondition(
+        dao.address,
+        ANY_ADDR,
+        PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID,
+        mockCondition.address
+      );
+
+      // Any contract can validate signatures now.
+      expect(await dao.connect(caller).isValidSignature(hash, goodSignature)).to
+        .not.be.reverted;
+      expect(await dao.connect(other).isValidSignature(hash, goodSignature)).to
+        .not.be.reverted;
+
+      // Because we use a mock condition always returning true, the signature is always valid.
       expect(
-        await dao.isValidSignature(ethers.utils.keccak256('0x00'), '0x00')
-      ).to.be.eq('0x41424344');
+        await dao.connect(caller).isValidSignature(hash, goodSignature)
+      ).to.equal(VALID_ERC1271_SIGNATURE);
+      expect(
+        await dao.connect(caller).isValidSignature(hash, badSignature)
+      ).to.equal(VALID_ERC1271_SIGNATURE);
     });
 
-    describe('ERC4824 - daoURI', async () => {
-      it('should set a new URI', async () => {
-        const newURI = 'https://new.example.com';
-        expect(await dao.daoURI()).not.to.be.eq(newURI);
-        await dao.setDaoURI(newURI);
-        expect(await dao.daoURI()).to.be.eq(newURI);
-      });
+    it('should revert if `setSignatureValidator` is called', async () => {
+      await expect(
+        dao
+          .connect(signers[2])
+          .setSignatureValidator(ethers.Wallet.createRandom().address)
+      ).to.be.revertedWithCustomError(dao, 'FunctionDeprecated');
+    });
+  });
 
-      it('should emit DaoURIUpdated', async () => {
-        const newURI = 'https://new.example.com';
-        await expect(dao.setDaoURI(newURI))
-          .to.emit(dao, DAO_EVENTS.NEW_URI)
-          .withArgs(newURI);
-      });
+  describe('ERC4824 - daoURI', async () => {
+    it('should set a new URI', async () => {
+      const newURI = 'https://new.example.com';
+      expect(await dao.daoURI()).not.to.be.eq(newURI);
+      await dao.setDaoURI(newURI);
+      expect(await dao.daoURI()).to.be.eq(newURI);
+    });
 
-      it('should revert if the sender lacks the permission to update the URI', async () => {
-        await dao.revoke(
+    it('should emit DaoURIUpdated', async () => {
+      const newURI = 'https://new.example.com';
+      await expect(dao.setDaoURI(newURI))
+        .to.emit(dao, DAO_EVENTS.NEW_URI)
+        .withArgs(newURI);
+    });
+
+    it('should revert if the sender lacks the permission to update the URI', async () => {
+      await dao.revoke(
+        dao.address,
+        ownerAddress,
+        PERMISSION_IDS.SET_METADATA_PERMISSION_ID
+      );
+
+      await expect(dao.setDaoURI('https://new.example.com'))
+        .to.be.revertedWithCustomError(dao, 'Unauthorized')
+        .withArgs(
           dao.address,
           ownerAddress,
           PERMISSION_IDS.SET_METADATA_PERMISSION_ID
         );
+    });
 
-        await expect(dao.setDaoURI('https://new.example.com'))
-          .to.be.revertedWithCustomError(dao, 'Unauthorized')
-          .withArgs(
-            dao.address,
-            ownerAddress,
-            PERMISSION_IDS.SET_METADATA_PERMISSION_ID
-          );
-      });
-
-      it('should return the DAO URI', async () => {
-        expect(await dao.daoURI()).to.be.eq(daoExampleURI);
-      });
+    it('should return the DAO URI', async () => {
+      expect(await dao.daoURI()).to.be.eq(daoExampleURI);
     });
   });
 });
diff --git a/packages/contracts/test/core/permission/permission-manager.ts b/packages/contracts/test/core/permission/permission-manager.ts
index 8d50a4374..3294a1753 100644
--- a/packages/contracts/test/core/permission/permission-manager.ts
+++ b/packages/contracts/test/core/permission/permission-manager.ts
@@ -21,15 +21,15 @@ const RESTRICTED_PERMISSIONS_FOR_ANY_ADDR = [
   ethers.utils.id('TEST_PERMISSION_2'),
 ];
 
-const UNSET_FLAG = ethers.utils.getAddress(
+export const UNSET_FLAG = ethers.utils.getAddress(
   '0x0000000000000000000000000000000000000000'
 );
-const ALLOW_FLAG = ethers.utils.getAddress(
+export const ALLOW_FLAG = ethers.utils.getAddress(
   '0x0000000000000000000000000000000000000002'
 );
+export const ANY_ADDR = '0xffffffffffffffffffffffffffffffffffffffff';
 
 const addressZero = ethers.constants.AddressZero;
-const ANY_ADDR = '0xffffffffffffffffffffffffffffffffffffffff';
 
 let conditionMock: PermissionConditionMock;
 
diff --git a/packages/contracts/test/framework/dao/dao-factory.ts b/packages/contracts/test/framework/dao/dao-factory.ts
index cb5a9cef0..e11af5d77 100644
--- a/packages/contracts/test/framework/dao/dao-factory.ts
+++ b/packages/contracts/test/framework/dao/dao-factory.ts
@@ -421,14 +421,6 @@ describe('DAOFactory: ', function () {
         ALLOW_FLAG
       )
       .to.emit(daoContract, EVENTS.Granted)
-      .withArgs(
-        SET_SIGNATURE_VALIDATOR_PERMISSION_ID,
-        daoFactory.address,
-        dao,
-        dao,
-        ALLOW_FLAG
-      )
-      .to.emit(daoContract, EVENTS.Granted)
       .withArgs(
         SET_TRUSTED_FORWARDER_PERMISSION_ID,
         daoFactory.address,

From 44972c2fd55544998555546f497236d88552493c Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Wed, 23 Aug 2023 17:55:45 +0200
Subject: [PATCH 02/38] chore: maintained changelog

---
 packages/contracts/CHANGELOG.md | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/packages/contracts/CHANGELOG.md b/packages/contracts/CHANGELOG.md
index dc6685d33..3ec80079a 100644
--- a/packages/contracts/CHANGELOG.md
+++ b/packages/contracts/CHANGELOG.md
@@ -5,6 +5,16 @@ 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
+
+### Changed
+
+- Use the DAOs permission manager functionality and conditions to evaluate signatures.
+
+### Removed
+
+- Deprecated the `setSignatureValidator` function and `signatureValidator` variable in `DAO`.
+
 ## v1.3.0-rc0
 
 ### Added

From b8bcc407fa04d481bcc9b58374a602cf713e66b0 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Wed, 23 Aug 2023 17:58:35 +0200
Subject: [PATCH 03/38] test: test default behaviour when no permission is
 granted

---
 packages/contracts/test/core/dao/dao.ts | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index 40165752e..f06da3004 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -1182,6 +1182,19 @@ describe('DAO', function () {
       ).to.be.eq(true);
     });
 
+    it('reverts all signatures requests by default', async () => {
+      const caller = signers[0];
+      const signer = signers[1];
+
+      const message = 'Hi!';
+      const hash = ethers.utils.hashMessage(message);
+      const signature = await signer.signMessage(message);
+
+      // The caller can validate signatures now.
+      expect(await dao.connect(caller).isValidSignature(hash, signature)).to.be
+        .reverted;
+    });
+
     it('allows caller specific signature validation', async () => {
       const caller = signers[0];
       const signer = signers[1];

From 8d94efcbc439d4cfad65cb6f56150d12e1e24ec2 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Thu, 24 Aug 2023 13:27:05 +0200
Subject: [PATCH 04/38] refactor: removed unused ERC1271Mock contract

---
 packages/contracts/src/test/dao/ERC1271Mock.sol | 9 ---------
 1 file changed, 9 deletions(-)
 delete mode 100644 packages/contracts/src/test/dao/ERC1271Mock.sol

diff --git a/packages/contracts/src/test/dao/ERC1271Mock.sol b/packages/contracts/src/test/dao/ERC1271Mock.sol
deleted file mode 100644
index cc39cfd91..000000000
--- a/packages/contracts/src/test/dao/ERC1271Mock.sol
+++ /dev/null
@@ -1,9 +0,0 @@
-// SPDX-License-Identifier: AGPL-3.0-or-later
-
-pragma solidity 0.8.17;
-
-contract ERC1271Mock {
-    function isValidSignature(bytes32, bytes memory) public pure returns (bytes4) {
-        return 0x41424344;
-    }
-}

From 63d31a8a7b2d5e615d97b19bb822112e80f53022 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Thu, 24 Aug 2023 13:29:34 +0200
Subject: [PATCH 05/38] chore: maintained changelog

---
 packages/contracts/CHANGELOG.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/packages/contracts/CHANGELOG.md b/packages/contracts/CHANGELOG.md
index 3ec80079a..ee73e7998 100644
--- a/packages/contracts/CHANGELOG.md
+++ b/packages/contracts/CHANGELOG.md
@@ -7,12 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## v1.4.0
 
+### Added
+
 ### Changed
 
 - Use the DAOs permission manager functionality and conditions to evaluate signatures.
 
 ### Removed
 
+- Removed unused `ERC1271Mock` contract.
 - Deprecated the `setSignatureValidator` function and `signatureValidator` variable in `DAO`.
 
 ## v1.3.0-rc0

From 9cd233d625518996a7b9bb4d58697a3ca09ddf24 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Fri, 25 Aug 2023 10:19:57 +0200
Subject: [PATCH 06/38] chore: maintained changelog

---
 packages/contracts/CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/contracts/CHANGELOG.md b/packages/contracts/CHANGELOG.md
index ee73e7998..93f7c5f7b 100644
--- a/packages/contracts/CHANGELOG.md
+++ b/packages/contracts/CHANGELOG.md
@@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
-- Use the DAOs permission manager functionality and conditions to evaluate signatures.
+- Use the DAOs permission manager functionality to validate signatures.
 
 ### Removed
 

From 4a1420f427520e7fce7c076429a5faa346cfaa02 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Fri, 25 Aug 2023 10:48:42 +0200
Subject: [PATCH 07/38] refactor: deprecation of variables and functions

---
 packages/contracts/CHANGELOG.md                        |  3 +++
 .../docs/osx/01-how-it-works/01-core/01-dao/index.md   |  2 +-
 packages/contracts/src/core/dao/DAO.sol                |  7 ++++---
 packages/contracts/src/core/dao/IDAO.sol               | 10 ++++------
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/packages/contracts/CHANGELOG.md b/packages/contracts/CHANGELOG.md
index 93f7c5f7b..f52a43df8 100644
--- a/packages/contracts/CHANGELOG.md
+++ b/packages/contracts/CHANGELOG.md
@@ -9,8 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- Added the `FunctionDeprecated` error to `DAO`.
+
 ### Changed
 
+- Renamed the `signatureValidator` variable in `DAO` to `__deprecated`.
 - Use the DAOs permission manager functionality to validate signatures.
 
 ### Removed
diff --git a/packages/contracts/docs/osx/01-how-it-works/01-core/01-dao/index.md b/packages/contracts/docs/osx/01-how-it-works/01-core/01-dao/index.md
index 9dff4e774..7ca51869c 100644
--- a/packages/contracts/docs/osx/01-how-it-works/01-core/01-dao/index.md
+++ b/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 -->
 
diff --git a/packages/contracts/src/core/dao/DAO.sol b/packages/contracts/src/core/dao/DAO.sol
index f23a7d04e..c13c173ce 100644
--- a/packages/contracts/src/core/dao/DAO.sol
+++ b/packages/contracts/src/core/dao/DAO.sol
@@ -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 Deprecated variable that is left here to maintain the storage layout.
+    /// @dev Introducedd in v1.0.0. Deprecated in v1.4.0.
+    /// @custom:oz-renamed-from signatureValidator
+    address private __deprecated;
 
     /// @notice The address of the trusted forwarder verifying meta transactions.
     /// @dev Added in v1.0.0.
diff --git a/packages/contracts/src/core/dao/IDAO.sol b/packages/contracts/src/core/dao/IDAO.sol
index 72de15094..90854efa0 100644
--- a/packages/contracts/src/core/dao/IDAO.sol
+++ b/packages/contracts/src/core/dao/IDAO.sol
@@ -112,12 +112,6 @@ interface IDAO {
     /// @param forwarder the new forwarder address.
     event TrustedForwarderSet(address forwarder);
 
-    /// @notice Deprecated.
-    function setSignatureValidator(address) external;
-
-    /// @notice Deprecated.
-    event SignatureValidatorSet(address);
-
     /// @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`.
@@ -133,4 +127,8 @@ interface IDAO {
         bytes4 _callbackSelector,
         bytes4 _magicNumber
     ) external;
+
+    /// @notice Deprecated function being left here to not corrupt the IDAO interface ID.
+    /// @dev Introduced in v1.0.0. Deprecated in v1.4.0.
+    function setSignatureValidator(address) external;
 }

From 78d9b64e0819ed31cd7bf532959498b5fdf44cad Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Fri, 25 Aug 2023 11:21:30 +0200
Subject: [PATCH 08/38] chore: bump protocol version to v1.4.0

---
 packages/contracts/package.json               |  1 +
 .../contracts/scripts/osx-versions-aliases.ts |  5 +-
 packages/contracts/src/test/Migration.sol     | 16 +++++
 .../src/utils/protocol/ProtocolVersion.sol    |  2 +-
 packages/contracts/test/core/dao/dao.ts       | 28 ++++++++
 .../test/framework/dao/dao-registry.ts        | 32 ++++++++-
 .../framework/plugin/plugin-repo-registry.ts  | 34 ++++++++-
 .../test/framework/plugin/plugin-repo.ts      | 28 ++++++++
 .../utils/ens/ens-subdomain-registry.ts       | 33 +++++++++
 .../addresslist/addresslist-voting.ts         | 32 +++++++++
 .../majority-voting/token/token-voting.ts     | 32 +++++++++
 .../plugins/governance/multisig/multisig.ts   | 32 +++++++++
 .../token/distribution/merkle-distributor.ts  | 31 +++++++++
 .../token/distribution/merkle-minter.ts       | 31 +++++++++
 .../test/test-utils/protocol-version.ts       |  2 +-
 packages/contracts/test/upgrade/dao.ts        | 69 ++++++++++---------
 yarn.lock                                     |  9 +++
 17 files changed, 381 insertions(+), 36 deletions(-)

diff --git a/packages/contracts/package.json b/packages/contracts/package.json
index 55f7339d8..38d8d207c 100644
--- a/packages/contracts/package.json
+++ b/packages/contracts/package.json
@@ -84,6 +84,7 @@
     "@openzeppelin/contracts": "4.8.1",
     "@openzeppelin/contracts-upgradeable": "4.8.1",
     "@aragon/osx-v1.0.1": "npm:@aragon/osx@1.0.1",
+    "@aragon/osx-v1.3.0-rc0.2": "npm:@aragon/osx@1.3.0-rc0.2",
     "@aragon/osx-ethers-v1.2.0": "npm:@aragon/osx-ethers@1.2.0"
   }
 }
diff --git a/packages/contracts/scripts/osx-versions-aliases.ts b/packages/contracts/scripts/osx-versions-aliases.ts
index 9c3601aec..03acd7956 100644
--- a/packages/contracts/scripts/osx-versions-aliases.ts
+++ b/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/',
+];
diff --git a/packages/contracts/src/test/Migration.sol b/packages/contracts/src/test/Migration.sol
index d5fdae2b2..15bf7188f 100644
--- a/packages/contracts/src/test/Migration.sol
+++ b/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";
diff --git a/packages/contracts/src/utils/protocol/ProtocolVersion.sol b/packages/contracts/src/utils/protocol/ProtocolVersion.sol
index f1fed8c48..8454d5043 100644
--- a/packages/contracts/src/utils/protocol/ProtocolVersion.sol
+++ b/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];
     }
 }
diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index 250068465..f0ee91be1 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -23,6 +23,7 @@ import {
   PermissionConditionMock__factory,
 } from '../../../typechain';
 import {DAO__factory as DAO_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/core/dao/DAO.sol';
+import {DAO__factory as DAO_V1_3_0__factory} from '../../../typechain/@aragon/osx-v1.3.0-rc0.2/core/dao/DAO.sol';
 
 import {
   getProtocolVersion,
@@ -387,6 +388,33 @@ describe('DAO', function () {
       );
       expect(toProtocolVersion).to.deep.equal(CURRENT_PROTOCOL_VERSION);
     });
+
+    it('from v1.3.0', async () => {
+      legacyContractFactory = new DAO_V1_3_0__factory(signers[0]);
+
+      const {fromImplementation, toImplementation} =
+        await deployAndUpgradeFromToCheck(
+          signers[0],
+          signers[1],
+          initArgs,
+          'initialize',
+          legacyContractFactory,
+          currentContractFactory,
+          UPGRADE_PERMISSIONS.UPGRADE_DAO_PERMISSION_ID
+        );
+      expect(toImplementation).to.not.equal(fromImplementation);
+
+      const fromProtocolVersion = await getProtocolVersion(
+        legacyContractFactory.attach(fromImplementation)
+      );
+      const toProtocolVersion = await getProtocolVersion(
+        currentContractFactory.attach(toImplementation)
+      );
+
+      expect(fromProtocolVersion).to.not.deep.equal(toProtocolVersion);
+      expect(fromProtocolVersion).to.deep.equal([1, 3, 0]);
+      expect(toProtocolVersion).to.deep.equal(CURRENT_PROTOCOL_VERSION);
+    });
   });
 
   describe('ERC-165', async () => {
diff --git a/packages/contracts/test/framework/dao/dao-registry.ts b/packages/contracts/test/framework/dao/dao-registry.ts
index c7cf860d5..53d59767a 100644
--- a/packages/contracts/test/framework/dao/dao-registry.ts
+++ b/packages/contracts/test/framework/dao/dao-registry.ts
@@ -10,6 +10,7 @@ import {
   ENSSubdomainRegistrar,
 } from '../../../typechain';
 import {DAORegistry__factory as DAORegistry_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/framework/dao/DAORegistry.sol';
+import {DAORegistry__factory as DAORegistry_V1_3_0__factory} from '../../../typechain/@aragon/osx-v1.3.0-rc0.2/framework/dao/DAORegistry.sol';
 
 import {deployNewDAO} from '../../test-utils/dao';
 import {deployENSSubdomainRegistrar} from '../../test-utils/ens';
@@ -295,7 +296,6 @@ describe('DAORegistry', function () {
           UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID,
           managingDao
         );
-
       expect(toImplementation).to.equal(fromImplementation); // The implementation was not changed from 1.0.0 to the current version
 
       const fromProtocolVersion = await getProtocolVersion(
@@ -311,5 +311,35 @@ describe('DAORegistry', function () {
       );
       expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
     });
+
+    it('from v1.3.0', async () => {
+      legacyContractFactory = new DAORegistry_V1_3_0__factory(signers[0]);
+
+      const {fromImplementation, toImplementation} =
+        await deployAndUpgradeFromToCheck(
+          signers[0],
+          signers[1],
+          initArgs,
+          'initialize',
+          legacyContractFactory,
+          currentContractFactory,
+          UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID,
+          managingDao
+        );
+      expect(toImplementation).to.equal(fromImplementation); // The implementation was not changed from 1.3.0 to the current version
+
+      const fromProtocolVersion = await getProtocolVersion(
+        legacyContractFactory.attach(fromImplementation)
+      );
+      const toProtocolVersion = await getProtocolVersion(
+        currentContractFactory.attach(toImplementation)
+      );
+
+      expect(fromProtocolVersion).to.deep.equal(toProtocolVersion);
+      expect(fromProtocolVersion).to.deep.equal(
+        IMPLICIT_INITIAL_PROTOCOL_VERSION
+      );
+      expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
+    });
   });
 });
diff --git a/packages/contracts/test/framework/plugin/plugin-repo-registry.ts b/packages/contracts/test/framework/plugin/plugin-repo-registry.ts
index 0dae6bcff..cd4ed9a3c 100644
--- a/packages/contracts/test/framework/plugin/plugin-repo-registry.ts
+++ b/packages/contracts/test/framework/plugin/plugin-repo-registry.ts
@@ -11,6 +11,7 @@ import {
   PluginRepoRegistry__factory,
 } from '../../../typechain';
 import {PluginRepoRegistry__factory as PluginRepoRegistry_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepoRegistry.sol';
+import {PluginRepoRegistry__factory as PluginRepoRegistry_V1_3_0__factory} from '../../../typechain/@aragon/osx-v1.3.0-rc0.2/framework/plugin/repo/PluginRepoRegistry.sol';
 
 import {deployNewDAO} from '../../test-utils/dao';
 import {deployNewPluginRepo} from '../../test-utils/repo';
@@ -306,7 +307,6 @@ describe('PluginRepoRegistry', function () {
           UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID,
           managingDAO
         );
-
       expect(toImplementation).to.equal(fromImplementation); // The implementation was not changed from 1.0.0 to the current version
 
       const fromProtocolVersion = await getProtocolVersion(
@@ -322,5 +322,37 @@ describe('PluginRepoRegistry', function () {
       );
       expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
     });
+
+    it('from v1.3.0', async () => {
+      legacyContractFactory = new PluginRepoRegistry_V1_3_0__factory(
+        signers[0]
+      );
+
+      const {fromImplementation, toImplementation} =
+        await deployAndUpgradeFromToCheck(
+          signers[0],
+          signers[1],
+          initArgs,
+          'initialize',
+          legacyContractFactory,
+          currentContractFactory,
+          UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID,
+          managingDAO
+        );
+      expect(toImplementation).to.equal(fromImplementation); // The implementation was not changed from 1.3.0 to the current version
+
+      const fromProtocolVersion = await getProtocolVersion(
+        legacyContractFactory.attach(fromImplementation)
+      );
+      const toProtocolVersion = await getProtocolVersion(
+        currentContractFactory.attach(toImplementation)
+      );
+
+      expect(fromProtocolVersion).to.deep.equal(toProtocolVersion);
+      expect(fromProtocolVersion).to.deep.equal(
+        IMPLICIT_INITIAL_PROTOCOL_VERSION
+      );
+      expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
+    });
   });
 });
diff --git a/packages/contracts/test/framework/plugin/plugin-repo.ts b/packages/contracts/test/framework/plugin/plugin-repo.ts
index 5853ef965..601cce3f4 100644
--- a/packages/contracts/test/framework/plugin/plugin-repo.ts
+++ b/packages/contracts/test/framework/plugin/plugin-repo.ts
@@ -17,6 +17,7 @@ import {
   IProtocolVersion__factory,
 } from '../../../typechain';
 import {PluginRepo__factory as PluginRepo_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepo.sol';
+import {PluginRepo__factory as PluginRepo_V1_3_0__factory} from '../../../typechain/@aragon/osx-v1.3.0-rc0.2/framework/plugin/repo/PluginRepo.sol';
 
 import {
   getProtocolVersion,
@@ -133,6 +134,33 @@ describe('PluginRepo', function () {
         );
         expect(toProtocolVersion).to.deep.equal(CURRENT_PROTOCOL_VERSION);
       });
+
+      it('from v1.3.0', async () => {
+        legacyContractFactory = new PluginRepo_V1_3_0__factory(signers[0]);
+
+        const {fromImplementation, toImplementation} =
+          await deployAndUpgradeFromToCheck(
+            signers[0],
+            signers[1],
+            initArgs,
+            'initialize',
+            legacyContractFactory,
+            currentContractFactory,
+            UPGRADE_PERMISSIONS.UPGRADE_REPO_PERMISSION_ID
+          );
+        expect(toImplementation).to.not.equal(fromImplementation);
+
+        const fromProtocolVersion = await getProtocolVersion(
+          legacyContractFactory.attach(fromImplementation)
+        );
+        const toProtocolVersion = await getProtocolVersion(
+          currentContractFactory.attach(toImplementation)
+        );
+
+        expect(fromProtocolVersion).to.not.deep.equal(toProtocolVersion);
+        expect(fromProtocolVersion).to.deep.equal([1, 3, 0]);
+        expect(toProtocolVersion).to.deep.equal(CURRENT_PROTOCOL_VERSION);
+      });
     });
 
     describe('ERC-165', async () => {
diff --git a/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts b/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts
index 0997d6985..3b4ea2003 100644
--- a/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts
+++ b/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts
@@ -13,6 +13,7 @@ import {
   ENSSubdomainRegistrar__factory,
 } from '../../../../typechain';
 import {ENSSubdomainRegistrar__factory as ENSSubdomainRegistrar_V1_0_0__factory} from '../../../../typechain/@aragon/osx-v1.0.1/framework/utils/ens/ENSSubdomainRegistrar.sol';
+import {ENSSubdomainRegistrar__factory as ENSSubdomainRegistrar_V1_3_0__factory} from '../../../../typechain/@aragon/osx-v1.3.0-rc0.2/framework/utils/ens/ENSSubdomainRegistrar.sol';
 
 import {deployWithProxy} from '../../../test-utils/proxy';
 import {deployNewDAO} from '../../../test-utils/dao';
@@ -351,6 +352,38 @@ describe('ENSSubdomainRegistrar', function () {
       );
       expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
     });
+
+    it('from v1.3.0', async () => {
+      legacyContractFactory = new ENSSubdomainRegistrar_V1_3_0__factory(
+        signers[0]
+      );
+
+      const {fromImplementation, toImplementation} =
+        await deployAndUpgradeFromToCheck(
+          signers[0],
+          signers[1],
+          initArgs,
+          'initialize',
+          legacyContractFactory,
+          currentContractFactory,
+          UPGRADE_PERMISSIONS.UPGRADE_REGISTRAR_PERMISSION_ID,
+          managingDao
+        );
+      expect(toImplementation).to.equal(fromImplementation); // The implementation was not changed from 1.3.0 to the current version
+
+      const fromProtocolVersion = await getProtocolVersion(
+        legacyContractFactory.attach(fromImplementation)
+      );
+      const toProtocolVersion = await getProtocolVersion(
+        currentContractFactory.attach(toImplementation)
+      );
+
+      expect(fromProtocolVersion).to.deep.equal(toProtocolVersion);
+      expect(fromProtocolVersion).to.deep.equal(
+        IMPLICIT_INITIAL_PROTOCOL_VERSION
+      );
+      expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
+    });
   });
 
   function expectedReverts() {
diff --git a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts
index 3be84438f..b8984ea45 100644
--- a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts
+++ b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts
@@ -16,6 +16,8 @@ import {
   IProposal__factory,
 } from '../../../../../typechain';
 import {AddresslistVoting__factory as AddresslistVoting_V1_0_0__factory} from '../../../../../typechain/@aragon/osx-v1.0.1/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol';
+import {AddresslistVoting__factory as AddresslistVoting_V1_3_0__factory} from '../../../../../typechain/@aragon/osx-v1.3.0-rc0.2/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol';
+
 import {
   ProposalCreatedEvent,
   ProposalExecutedEvent,
@@ -191,6 +193,36 @@ describe('AddresslistVoting', function () {
       );
       expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
     });
+
+    it('from v1.3.0', async () => {
+      legacyContractFactory = new AddresslistVoting_V1_3_0__factory(signers[0]);
+
+      const {fromImplementation, toImplementation} =
+        await deployAndUpgradeFromToCheck(
+          signers[0],
+          signers[1],
+
+          initArgs,
+          'initialize',
+          legacyContractFactory,
+          currentContractFactory,
+          UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID,
+          dao
+        );
+      expect(toImplementation).to.equal(fromImplementation);
+
+      const fromProtocolVersion = await getProtocolVersion(
+        legacyContractFactory.attach(fromImplementation)
+      );
+      const toProtocolVersion = await getProtocolVersion(
+        currentContractFactory.attach(toImplementation)
+      );
+      expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.3.0 to the current version
+      expect(fromProtocolVersion).to.deep.equal(
+        IMPLICIT_INITIAL_PROTOCOL_VERSION
+      );
+      expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
+    });
   });
 
   describe('plugin interface: ', async () => {
diff --git a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts
index 71d854eea..54e2e852a 100644
--- a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts
+++ b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts
@@ -17,6 +17,8 @@ import {
   TokenVoting__factory,
 } from '../../../../../typechain';
 import {TokenVoting__factory as TokenVoting_V1_0_0__factory} from '../../../../../typechain/@aragon/osx-v1.0.1/plugins/governance/majority-voting/token/TokenVoting.sol';
+import {TokenVoting__factory as TokenVoting_V1_3_0__factory} from '../../../../../typechain/@aragon/osx-v1.3.0-rc0.2/plugins/governance/majority-voting/token/TokenVoting.sol';
+
 import {
   ProposalCreatedEvent,
   ProposalExecutedEvent,
@@ -258,6 +260,36 @@ describe('TokenVoting', function () {
       );
       expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
     });
+
+    it('from v1.3.0', async () => {
+      legacyContractFactory = new TokenVoting_V1_3_0__factory(signers[0]);
+
+      const {fromImplementation, toImplementation} =
+        await deployAndUpgradeFromToCheck(
+          signers[0],
+          signers[1],
+
+          initArgs,
+          'initialize',
+          legacyContractFactory,
+          currentContractFactory,
+          UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID,
+          dao
+        );
+      expect(toImplementation).to.equal(fromImplementation);
+
+      const fromProtocolVersion = await getProtocolVersion(
+        legacyContractFactory.attach(fromImplementation)
+      );
+      const toProtocolVersion = await getProtocolVersion(
+        currentContractFactory.attach(toImplementation)
+      );
+      expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.3.0 to the current version
+      expect(fromProtocolVersion).to.deep.equal(
+        IMPLICIT_INITIAL_PROTOCOL_VERSION
+      );
+      expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
+    });
   });
 
   describe('plugin interface: ', async () => {
diff --git a/packages/contracts/test/plugins/governance/multisig/multisig.ts b/packages/contracts/test/plugins/governance/multisig/multisig.ts
index fb63f0c1b..fb9783bf8 100644
--- a/packages/contracts/test/plugins/governance/multisig/multisig.ts
+++ b/packages/contracts/test/plugins/governance/multisig/multisig.ts
@@ -16,6 +16,8 @@ import {
   Multisig__factory,
 } from '../../../../typechain';
 import {Multisig__factory as Multisig_V1_0_0__factory} from '../../../../typechain/@aragon/osx-v1.0.1/plugins/governance/multisig/Multisig.sol';
+import {Multisig__factory as Multisig_V1_3_0__factory} from '../../../../typechain/@aragon/osx-v1.3.0-rc0.2/plugins/governance/multisig/Multisig.sol';
+
 import {
   ApprovedEvent,
   ProposalExecutedEvent,
@@ -258,6 +260,36 @@ describe('Multisig', function () {
       );
       expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
     });
+
+    it('from v1.3.0', async () => {
+      legacyContractFactory = new Multisig_V1_3_0__factory(signers[0]);
+
+      const {fromImplementation, toImplementation} =
+        await deployAndUpgradeFromToCheck(
+          signers[0],
+          signers[1],
+          initArgs,
+          'initialize',
+          legacyContractFactory,
+          currentContractFactory,
+          UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID,
+          dao
+        );
+      expect(toImplementation).to.equal(fromImplementation);
+
+      const fromProtocolVersion = await getProtocolVersion(
+        legacyContractFactory.attach(fromImplementation)
+      );
+      const toProtocolVersion = await getProtocolVersion(
+        currentContractFactory.attach(toImplementation)
+      );
+
+      expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.3.0 to the current version
+      expect(fromProtocolVersion).to.deep.equal(
+        IMPLICIT_INITIAL_PROTOCOL_VERSION
+      );
+      expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
+    });
   });
 
   describe('plugin interface: ', async () => {
diff --git a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts
index f7359c39e..5b34c1821 100644
--- a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts
+++ b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts
@@ -16,6 +16,7 @@ import {
   MerkleDistributor__factory,
 } from '../../../../typechain';
 import {MerkleDistributor__factory as MerkleDistributor_V1_0_0__factory} from '../../../../typechain/@aragon/osx-v1.0.1/plugins/token/MerkleDistributor.sol';
+import {MerkleDistributor__factory as MerkleDistributor_V1_3_0__factory} from '../../../../typechain/@aragon/osx-v1.3.0-rc0.2/plugins/token/MerkleDistributor.sol';
 
 import {deployWithProxy} from '../../../test-utils/proxy';
 import BalanceTree from './src/balance-tree';
@@ -141,6 +142,36 @@ describe('MerkleDistributor', function () {
       );
       expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
     });
+
+    it('from v1.3.0', async () => {
+      legacyContractFactory = new MerkleDistributor_V1_3_0__factory(signers[0]);
+
+      const {fromImplementation, toImplementation} =
+        await deployAndUpgradeFromToCheck(
+          signers[0],
+          signers[1],
+          initArgs,
+          'initialize',
+          legacyContractFactory,
+          currentContractFactory,
+          UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID,
+          dao
+        );
+      expect(toImplementation).to.equal(fromImplementation); // The build did not change
+
+      const fromProtocolVersion = await getProtocolVersion(
+        legacyContractFactory.attach(fromImplementation)
+      );
+      const toProtocolVersion = await getProtocolVersion(
+        currentContractFactory.attach(toImplementation)
+      );
+
+      expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.3.0 to the current version
+      expect(fromProtocolVersion).to.deep.equal(
+        IMPLICIT_INITIAL_PROTOCOL_VERSION
+      );
+      expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
+    });
   });
 
   describe('general', () => {
diff --git a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts
index cb462db30..026c3cd37 100644
--- a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts
+++ b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts
@@ -18,6 +18,7 @@ import {
   GovernanceERC20__factory,
 } from '../../../../typechain';
 import {MerkleMinter__factory as MerkleMinter_V1_0_0__factory} from '../../../../typechain/@aragon/osx-v1.0.1/plugins/token/MerkleMinter.sol';
+import {MerkleMinter__factory as MerkleMinter_V1_3_0__factory} from '../../../../typechain/@aragon/osx-v1.3.0-rc0.2/plugins/token/MerkleMinter.sol';
 
 import BalanceTree from './src/balance-tree';
 import {deployNewDAO} from '../../../test-utils/dao';
@@ -151,6 +152,36 @@ describe('MerkleMinter', function () {
       );
       expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
     });
+
+    it('from v1.3.0', async () => {
+      legacyContractFactory = new MerkleMinter_V1_3_0__factory(signers[0]);
+
+      const {fromImplementation, toImplementation} =
+        await deployAndUpgradeFromToCheck(
+          signers[0],
+          signers[1],
+          initArgs,
+          'initialize',
+          legacyContractFactory,
+          currentContractFactory,
+          UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID,
+          managingDao
+        );
+      expect(toImplementation).to.equal(fromImplementation); // The build did not change
+
+      const fromProtocolVersion = await getProtocolVersion(
+        legacyContractFactory.attach(fromImplementation)
+      );
+      const toProtocolVersion = await getProtocolVersion(
+        currentContractFactory.attach(toImplementation)
+      );
+
+      expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.3.0 to the current version
+      expect(fromProtocolVersion).to.deep.equal(
+        IMPLICIT_INITIAL_PROTOCOL_VERSION
+      );
+      expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION);
+    });
   });
 
   describe('plugin interface: ', async () => {
diff --git a/packages/contracts/test/test-utils/protocol-version.ts b/packages/contracts/test/test-utils/protocol-version.ts
index fe6b20723..28eca6de1 100644
--- a/packages/contracts/test/test-utils/protocol-version.ts
+++ b/packages/contracts/test/test-utils/protocol-version.ts
@@ -1,5 +1,5 @@
 // The current protocol version number as specified by the `getProtocolVersion()` function in `ProtocolVersion.sol`.
-export const CURRENT_PROTOCOL_VERSION: [number, number, number] = [1, 3, 0];
+export const CURRENT_PROTOCOL_VERSION: [number, number, number] = [1, 4, 0];
 
 // The protocol version number of contracts not having a `getProtocolVersion()` function because they don't inherit from `ProtocolVersion.sol` yet.
 export const IMPLICIT_INITIAL_PROTOCOL_VERSION: [number, number, number] = [
diff --git a/packages/contracts/test/upgrade/dao.ts b/packages/contracts/test/upgrade/dao.ts
index 406841ea0..b0d40269c 100644
--- a/packages/contracts/test/upgrade/dao.ts
+++ b/packages/contracts/test/upgrade/dao.ts
@@ -7,7 +7,12 @@ import {
   DAO__factory as DAO_V1_0_0__factory,
 } from '../../typechain/@aragon/osx-v1.0.1/core/dao/DAO.sol';
 
-import {DAO, DAO__factory, ProtocolVersion__factory} from '../../typechain';
+import {
+  DAO as DAO_V1_3_0,
+  DAO__factory as DAO_V1_3_0__factory,
+} from '../../typechain/@aragon/osx-v1.3.0-rc0.2/core/dao/DAO.sol';
+
+import {DAO, ProtocolVersion__factory} from '../../typechain';
 
 import {daoExampleURI, ZERO_BYTES32} from '../test-utils/dao';
 import {deployWithProxy} from '../test-utils/proxy';
@@ -19,12 +24,10 @@ import {UpgradedEvent} from '../../typechain/DAO';
 import {IMPLICIT_INITIAL_PROTOCOL_VERSION} from '../test-utils/protocol-version';
 
 let signers: SignerWithAddress[];
-let DAO_old: DAO_V1_0_0__factory;
-let DAO_Current: DAO__factory;
 
 let daoV100Proxy: DAO_V1_0_0;
-let daoV100Implementation: string;
-let daoCurrentImplementaion: DAO;
+let daoV100Implementation: DAO_V1_0_0;
+let daoV130Implementation: DAO_V1_3_0;
 
 const EMPTY_DATA = '0x';
 
@@ -39,18 +42,16 @@ describe('DAO Upgrade', function () {
   before(async function () {
     signers = await ethers.getSigners();
 
-    // We don't use the typchain here but directly grab the artifacts. This will be changed in an upcoming PR again.
-    DAO_old = new DAO_V1_0_0__factory(signers[0]);
-    DAO_Current = new DAO__factory(signers[0]);
-
     // Deploy the v1.3.0 implementation
-    daoCurrentImplementaion = await DAO_Current.deploy();
+    daoV130Implementation = await new DAO_V1_3_0__factory(signers[0]).deploy();
   });
 
   context(`Re-entrancy`, function () {
     context(`v1.0.0 to v1.3.0`, function () {
       beforeEach(async function () {
-        daoV100Proxy = await deployWithProxy<DAO_V1_0_0>(DAO_old);
+        daoV100Proxy = await deployWithProxy<DAO_V1_0_0>(
+          new DAO_V1_0_0__factory(signers[0])
+        );
         await daoV100Proxy.initialize(
           DUMMY_METADATA,
           signers[0].address,
@@ -59,8 +60,8 @@ describe('DAO Upgrade', function () {
         );
 
         // Store the v1.0.0 implementation
-        daoV100Implementation = await readImplementationValueFromSlot(
-          daoV100Proxy.address
+        daoV100Implementation = new DAO_V1_0_0__factory(signers[0]).attach(
+          await readImplementationValueFromSlot(daoV100Proxy.address)
         );
 
         // Grant the upgrade permission
@@ -74,8 +75,8 @@ describe('DAO Upgrade', function () {
       it('does not corrupt the DAO storage', async () => {
         // Upgrade and call `initializeFrom`.
         const upgradeTx = await daoV100Proxy.upgradeToAndCall(
-          daoCurrentImplementaion.address,
-          DAO_Current.interface.encodeFunctionData('initializeFrom', [
+          daoV130Implementation.address,
+          daoV130Implementation.interface.encodeFunctionData('initializeFrom', [
             IMPLICIT_INITIAL_PROTOCOL_VERSION,
             EMPTY_DATA,
           ])
@@ -85,7 +86,7 @@ describe('DAO Upgrade', function () {
         const implementationAfterUpgrade =
           await readImplementationValueFromSlot(daoV100Proxy.address);
         expect(implementationAfterUpgrade).to.equal(
-          daoCurrentImplementaion.address
+          daoV130Implementation.address
         );
         expect(implementationAfterUpgrade).to.not.equal(daoV100Implementation);
 
@@ -93,11 +94,11 @@ describe('DAO Upgrade', function () {
         const emittedImplementation = (
           await findEventTopicLog<UpgradedEvent>(
             upgradeTx,
-            DAO_old.interface,
+            daoV130Implementation.interface,
             'Upgraded'
           )
         ).args.implementation;
-        expect(emittedImplementation).to.equal(daoCurrentImplementaion.address);
+        expect(emittedImplementation).to.equal(daoV130Implementation.address);
 
         // Check that storage is not corrupted.
         expect(await daoV100Proxy.callStatic.daoURI()).to.equal(daoExampleURI);
@@ -140,8 +141,8 @@ describe('DAO Upgrade', function () {
 
         // Upgrade and call `initializeFrom`.
         await daoV100Proxy.upgradeToAndCall(
-          daoCurrentImplementaion.address,
-          DAO_Current.interface.encodeFunctionData('initializeFrom', [
+          daoV130Implementation.address,
+          daoV130Implementation.interface.encodeFunctionData('initializeFrom', [
             IMPLICIT_INITIAL_PROTOCOL_VERSION,
             EMPTY_DATA,
           ])
@@ -151,7 +152,7 @@ describe('DAO Upgrade', function () {
         const implementationAfterUpgrade =
           await readImplementationValueFromSlot(daoV100Proxy.address);
         expect(implementationAfterUpgrade).to.equal(
-          daoCurrentImplementaion.address
+          daoV130Implementation.address
         );
         expect(implementationAfterUpgrade).to.not.equal(daoV100Implementation);
 
@@ -220,8 +221,8 @@ describe('DAO Upgrade', function () {
 
         // Upgrade and call `initializeFrom`.
         await daoV100Proxy.upgradeToAndCall(
-          daoCurrentImplementaion.address,
-          DAO_Current.interface.encodeFunctionData('initializeFrom', [
+          daoV130Implementation.address,
+          daoV130Implementation.interface.encodeFunctionData('initializeFrom', [
             IMPLICIT_INITIAL_PROTOCOL_VERSION,
             EMPTY_DATA,
           ])
@@ -231,7 +232,7 @@ describe('DAO Upgrade', function () {
         const implementationAfterUpgrade =
           await readImplementationValueFromSlot(daoV100Proxy.address);
         expect(implementationAfterUpgrade).to.equal(
-          daoCurrentImplementaion.address
+          daoV130Implementation.address
         );
         expect(implementationAfterUpgrade).to.not.equal(daoV100Implementation);
 
@@ -262,7 +263,9 @@ describe('DAO Upgrade', function () {
   context(`Protocol Version`, function () {
     beforeEach(async function () {
       // prepare v1.0.0
-      daoV100Proxy = await deployWithProxy<DAO_V1_0_0>(DAO_old);
+      daoV100Proxy = await deployWithProxy<DAO_V1_0_0>(
+        new DAO_V1_0_0__factory(signers[0])
+      );
       await daoV100Proxy.initialize(
         DUMMY_METADATA,
         signers[0].address,
@@ -280,7 +283,9 @@ describe('DAO Upgrade', function () {
 
     it('fails to call protocolVersion on versions prior to v1.3.0 and succeeds from v1.3.0 onwards', async () => {
       // deploy the different versions
-      const daoCurrentProxy = await deployWithProxy<DAO>(DAO_Current);
+      const daoCurrentProxy = await deployWithProxy<DAO>(
+        new DAO_V1_3_0__factory(signers[0])
+      );
       await daoCurrentProxy.initialize(
         DUMMY_METADATA,
         signers[0].address,
@@ -325,8 +330,8 @@ describe('DAO Upgrade', function () {
 
         // Upgrade and call `initializeFrom`.
         await daoV100Proxy.upgradeToAndCall(
-          daoCurrentImplementaion.address,
-          DAO_Current.interface.encodeFunctionData('initializeFrom', [
+          daoV130Implementation.address,
+          daoV130Implementation.interface.encodeFunctionData('initializeFrom', [
             IMPLICIT_INITIAL_PROTOCOL_VERSION,
             EMPTY_DATA,
           ])
@@ -343,14 +348,16 @@ describe('DAO Upgrade', function () {
       it('returns the correct protocol version after upgrade', async () => {
         // Upgrade and call `initializeFrom`.
         await daoV100Proxy.upgradeToAndCall(
-          daoCurrentImplementaion.address,
-          DAO_Current.interface.encodeFunctionData('initializeFrom', [
+          daoV130Implementation.address,
+          daoV130Implementation.interface.encodeFunctionData('initializeFrom', [
             IMPLICIT_INITIAL_PROTOCOL_VERSION,
             EMPTY_DATA,
           ])
         );
 
-        const daoV130 = DAO_Current.attach(daoV100Proxy.address);
+        const daoV130 = new DAO_V1_3_0__factory(signers[0]).attach(
+          daoV100Proxy.address
+        );
         expect(await daoV130.protocolVersion()).to.be.deep.eq([1, 3, 0]);
       });
     });
diff --git a/yarn.lock b/yarn.lock
index 70e15a283..6ee7ef98d 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -14,6 +14,15 @@
   resolved "https://registry.yarnpkg.com/@aragon/osx/-/osx-1.0.1.tgz#b758ba87db93a46a8ddabfaefc99ac8e44c46c78"
   integrity sha512-TiP5/1AGv/hth+V8PoDVFlwMzmLazYxzp//jiepAZ0WJkx9EnQNYafo+M7+pjAqRPG005liQjmFZNiK6ARLULg==
 
+"@aragon/osx-v1.3.0-rc0.2@npm:@aragon/osx@1.3.0-rc0.2":
+  version "1.3.0-rc0.2"
+  resolved "https://registry.yarnpkg.com/@aragon/osx/-/osx-1.3.0-rc0.2.tgz#6922052bc7eaa180e143478b56292cc1e1245adf"
+  integrity sha512-IQTOPXgISEOGhcf2LeE4iz+TF4YmNzYjga9UgaIEYpcMkVKN1LUdJgvrF6VxGeGWiDjRTb0nNijtdmeOw4ungw==
+  dependencies:
+    "@ensdomains/ens-contracts" "0.0.11"
+    "@openzeppelin/contracts" "4.8.1"
+    "@openzeppelin/contracts-upgradeable" "4.8.1"
+
 "@babel/code-frame@7.12.11":
   version "7.12.11"
   resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.12.11.tgz#f4ad435aa263db935b8f10f2c552d23fb716a63f"

From 1011871b02b0a7fe75966bc3c3008ad07f2c19d2 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Fri, 25 Aug 2023 11:58:15 +0200
Subject: [PATCH 09/38] chore: maintained changelog

---
 packages/contracts/CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/packages/contracts/CHANGELOG.md b/packages/contracts/CHANGELOG.md
index f52a43df8..de6b29dad 100644
--- a/packages/contracts/CHANGELOG.md
+++ b/packages/contracts/CHANGELOG.md
@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Removed
 
+- Removed the `SignatureValidatorSet` event from `IDAO`.
 - Removed unused `ERC1271Mock` contract.
 - Deprecated the `setSignatureValidator` function and `signatureValidator` variable in `DAO`.
 

From aca7c5ec35d474a5fd3493b1b66ce323acad6ee1 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Fri, 25 Aug 2023 16:32:03 +0200
Subject: [PATCH 10/38] refactor: use removed instead of depreacted

---
 packages/contracts/CHANGELOG.md          |  2 +-
 packages/contracts/src/core/dao/DAO.sol  | 12 ++++++------
 packages/contracts/src/core/dao/IDAO.sol |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/packages/contracts/CHANGELOG.md b/packages/contracts/CHANGELOG.md
index de6b29dad..9047410de 100644
--- a/packages/contracts/CHANGELOG.md
+++ b/packages/contracts/CHANGELOG.md
@@ -20,7 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Removed the `SignatureValidatorSet` event from `IDAO`.
 - Removed unused `ERC1271Mock` contract.
-- Deprecated the `setSignatureValidator` function and `signatureValidator` variable in `DAO`.
+- 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.0-rc0
 
diff --git a/packages/contracts/src/core/dao/DAO.sol b/packages/contracts/src/core/dao/DAO.sol
index 48ac37a0c..5048bf880 100644
--- a/packages/contracts/src/core/dao/DAO.sol
+++ b/packages/contracts/src/core/dao/DAO.sol
@@ -69,10 +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 Deprecated variable that is left here to maintain the storage layout.
-    /// @dev Introducedd in v1.0.0. Deprecated in v1.4.0.
+    /// @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 __deprecated;
+    address private __removed1;
 
     /// @notice The address of the trusted forwarder verifying meta transactions.
     /// @dev Added in v1.0.0.
@@ -110,8 +110,8 @@ 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 deprecated.
-    error FunctionDeprecated();
+    /// @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.
@@ -324,7 +324,7 @@ contract DAO is
 
     /// @inheritdoc IDAO
     function setSignatureValidator(address) external pure override {
-        revert FunctionDeprecated();
+        revert FunctionRemoved();
     }
 
     /// @inheritdoc IDAO
diff --git a/packages/contracts/src/core/dao/IDAO.sol b/packages/contracts/src/core/dao/IDAO.sol
index 90854efa0..5b6c0d233 100644
--- a/packages/contracts/src/core/dao/IDAO.sol
+++ b/packages/contracts/src/core/dao/IDAO.sol
@@ -128,7 +128,7 @@ interface IDAO {
         bytes4 _magicNumber
     ) external;
 
-    /// @notice Deprecated function being left here to not corrupt the IDAO interface ID.
-    /// @dev Introduced in v1.0.0. Deprecated in v1.4.0.
+    /// @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;
 }

From 33d144f4cd43733fea832c3b0d271e5e6acbd409 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Mon, 28 Aug 2023 08:52:50 +0200
Subject: [PATCH 11/38] chore: bump package versions

---
 packages/contracts-ethers/package.json | 2 +-
 packages/contracts/package.json        | 2 +-
 packages/contracts/src/package.json    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/packages/contracts-ethers/package.json b/packages/contracts-ethers/package.json
index a62f68fda..bebf7e6b5 100644
--- a/packages/contracts-ethers/package.json
+++ b/packages/contracts-ethers/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@aragon/osx-ethers",
-  "version": "1.3.0-rc0.2",
+  "version": "1.4.0-rc0",
   "description": "The Aragon OSx contract definitions for ethers.js",
   "main": "dist/bundle-cjs.js",
   "module": "dist/bundle-esm.js",
diff --git a/packages/contracts/package.json b/packages/contracts/package.json
index 4f73c55b4..0af486dba 100644
--- a/packages/contracts/package.json
+++ b/packages/contracts/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@aragon/osx-artifacts",
-  "version": "1.3.0-rc0.2",
+  "version": "1.4.0-rc0",
   "description": "The Aragon OSx Solidity contracts",
   "main": "dist/bundle-cjs.js",
   "module": "dist/bundle-esm.js",
diff --git a/packages/contracts/src/package.json b/packages/contracts/src/package.json
index 6d8a807af..1ef73d391 100644
--- a/packages/contracts/src/package.json
+++ b/packages/contracts/src/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@aragon/osx",
-  "version": "1.3.0-rc0.2",
+  "version": "1.4.0-rc0",
   "description": "The Aragon OSx Solidity contracts",
   "publishConfig": {
     "access": "public"

From ca012ccea9dc6397c9fce4e28a835ac9682fcd20 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Mon, 28 Aug 2023 09:53:24 +0200
Subject: [PATCH 12/38] fix: wrong error name

---
 packages/contracts/test/core/dao/dao.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index f0ee91be1..8fcc3e389 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -1361,7 +1361,7 @@ describe('DAO', function () {
         dao
           .connect(signers[2])
           .setSignatureValidator(ethers.Wallet.createRandom().address)
-      ).to.be.revertedWithCustomError(dao, 'FunctionDeprecated');
+      ).to.be.revertedWithCustomError(dao, 'FunctionRemoved');
     });
   });
 

From 602e22c5c2e24be6f55dd266c5834e086ebc0512 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Mon, 28 Aug 2023 11:11:32 +0200
Subject: [PATCH 13/38] fix: wrong version number in changelog

---
 packages/contracts/CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/contracts/CHANGELOG.md b/packages/contracts/CHANGELOG.md
index 9047410de..572166300 100644
--- a/packages/contracts/CHANGELOG.md
+++ b/packages/contracts/CHANGELOG.md
@@ -5,7 +5,7 @@ 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
+## v1.4.0-rc0
 
 ### Added
 

From 11a3b002a33daf79502625468d8730fbc8cacba0 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Wed, 30 Aug 2023 12:33:43 +0200
Subject: [PATCH 14/38] test: fixed broken test

---
 packages/contracts/test/core/dao/dao.ts | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index 8fcc3e389..82320c083 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -141,11 +141,6 @@ describe('DAO', function () {
         ownerAddress,
         PERMISSION_IDS.UPGRADE_DAO_PERMISSION_ID
       ),
-      dao.grant(
-        dao.address,
-        ownerAddress,
-        PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID
-      ),
       dao.grant(
         dao.address,
         ownerAddress,
@@ -1230,7 +1225,7 @@ describe('DAO', function () {
       ).to.be.eq(true);
     });
 
-    it('reverts all signatures requests by default', async () => {
+    it('treats signatures as invalid by default if no permission is set', async () => {
       const caller = signers[0];
       const signer = signers[1];
 
@@ -1238,9 +1233,10 @@ describe('DAO', function () {
       const hash = ethers.utils.hashMessage(message);
       const signature = await signer.signMessage(message);
 
-      // The caller can validate signatures now.
-      expect(await dao.connect(caller).isValidSignature(hash, signature)).to.be
-        .reverted;
+      // Because no permission is set, the signature is said to be invalid
+      expect(
+        await dao.connect(caller).isValidSignature(hash, signature)
+      ).to.equal(INVALID_ERC1271_SIGNATURE);
     });
 
     it('allows caller specific signature validation', async () => {

From e82dc3ac48ee04c29cb0c875122ed702655baec3 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Thu, 31 Aug 2023 18:58:22 +0200
Subject: [PATCH 15/38] revert: do not export flags that are not needed in
 other files currently

---
 packages/contracts/test/core/permission/permission-manager.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/contracts/test/core/permission/permission-manager.ts b/packages/contracts/test/core/permission/permission-manager.ts
index 3294a1753..9824ec0ea 100644
--- a/packages/contracts/test/core/permission/permission-manager.ts
+++ b/packages/contracts/test/core/permission/permission-manager.ts
@@ -21,10 +21,10 @@ const RESTRICTED_PERMISSIONS_FOR_ANY_ADDR = [
   ethers.utils.id('TEST_PERMISSION_2'),
 ];
 
-export const UNSET_FLAG = ethers.utils.getAddress(
+const UNSET_FLAG = ethers.utils.getAddress(
   '0x0000000000000000000000000000000000000000'
 );
-export const ALLOW_FLAG = ethers.utils.getAddress(
+const ALLOW_FLAG = ethers.utils.getAddress(
   '0x0000000000000000000000000000000000000002'
 );
 export const ANY_ADDR = '0xffffffffffffffffffffffffffffffffffffffff';

From 80df2d318031c32e45f3bcf0b7d729194b8ce73c Mon Sep 17 00:00:00 2001
From: Michael Heuer <20623991+Michael-A-Heuer@users.noreply.github.com>
Date: Fri, 1 Sep 2023 13:32:25 +0200
Subject: [PATCH 16/38] Apply suggestions from code review

Co-authored-by: Mathias Scherer <mathias@aragon.org>
---
 packages/contracts/CHANGELOG.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/contracts/CHANGELOG.md b/packages/contracts/CHANGELOG.md
index 1844efb1f..401b7b0f0 100644
--- a/packages/contracts/CHANGELOG.md
+++ b/packages/contracts/CHANGELOG.md
@@ -9,11 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
-- Added the `FunctionDeprecated` error to `DAO`.
+- Added the `FunctionRemoved` error to `DAO`.
 
 ### Changed
 
-- Renamed the `signatureValidator` variable in `DAO` to `__deprecated`.
+- Renamed the `signatureValidator` variable in `DAO` to `__removed`.
 - Use the DAOs permission manager functionality to validate signatures.
 
 ### Removed

From d2d5a54e35ef47f3ad837be150884863281e4223 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Fri, 1 Sep 2023 14:39:00 +0200
Subject: [PATCH 17/38] chore: maintain changelog

---
 packages/contracts/CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/contracts/CHANGELOG.md b/packages/contracts/CHANGELOG.md
index 401b7b0f0..4fe77298d 100644
--- a/packages/contracts/CHANGELOG.md
+++ b/packages/contracts/CHANGELOG.md
@@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
-- Renamed the `signatureValidator` variable in `DAO` to `__removed`.
+- Renamed the `signatureValidator` variable in `DAO` to `__removed0`.
 - Use the DAOs permission manager functionality to validate signatures.
 
 ### Removed

From 25b7704e70b50f8b8a546560083e551d0aeab194 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Fri, 1 Sep 2023 15:58:45 +0200
Subject: [PATCH 18/38] refactor: improved confusing and broken tests

---
 .../permission/PermissionConditionMock.sol    |  25 +--
 packages/contracts/test/core/dao/dao.ts       | 155 +++++++++---------
 .../core/permission/permission-manager.ts     |   2 +-
 3 files changed, 90 insertions(+), 92 deletions(-)

diff --git a/packages/contracts/src/test/permission/PermissionConditionMock.sol b/packages/contracts/src/test/permission/PermissionConditionMock.sol
index 21b70db87..c9c27a275 100644
--- a/packages/contracts/src/test/permission/PermissionConditionMock.sol
+++ b/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;
     }
 }
diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index 82320c083..d61f53d63 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -1217,44 +1217,32 @@ describe('DAO', function () {
   });
 
   describe('ERC1271', async () => {
-    it('should register the interfaceId', async () => {
-      expect(
-        await dao.supportsInterface(
-          getInterfaceID(IERC1271__factory.createInterface())
-        )
-      ).to.be.eq(true);
-    });
+    let signer: SignerWithAddress;
+    let caller: SignerWithAddress;
+    let otherCaller: SignerWithAddress;
 
-    it('treats signatures as invalid by default if no permission is set', async () => {
-      const caller = signers[0];
-      const signer = signers[1];
+    let message: string;
+    let hash: string;
+    let signature: string;
 
-      const message = 'Hi!';
-      const hash = ethers.utils.hashMessage(message);
-      const signature = await signer.signMessage(message);
+    beforeEach(async () => {
+      caller = signers[0];
+      signer = signers[1];
+      otherCaller = signers[2];
+
+      message = 'The message!';
+      hash = ethers.utils.hashMessage(message);
+      signature = await signer.signMessage(message);
+    });
 
-      // Because no permission is set, the signature is said to be invalid
+    it('treats signatures as invalid by default if no permission is set', async () => {
       expect(
         await dao.connect(caller).isValidSignature(hash, signature)
       ).to.equal(INVALID_ERC1271_SIGNATURE);
     });
 
-    it('allows caller specific signature validation', async () => {
-      const caller = signers[0];
-      const signer = signers[1];
-
-      const message = 'Hi!';
-      const hash = ethers.utils.hashMessage(message);
-      const goodSignature = await signer.signMessage(message);
-      const badSignature = await signer.signMessage('Bye!');
-      expect(badSignature).to.not.equal(goodSignature);
-
-      // Try to call with caller but caller has no permission
-      expect(await dao.connect(caller).isValidSignature(hash, goodSignature))
-        .to.be.revertedWithCustomError(dao, 'Unauthorized')
-        .withArgs();
-
-      // Grant the permission to validate signatures to the caller unconditionally
+    it('allows caller-specific signature validation bypassing', async () => {
+      // Grant the permission to validate signatures to the caller without a condition
       await dao.grant(
         dao.address,
         caller.address,
@@ -1262,69 +1250,65 @@ describe('DAO', function () {
       );
 
       // The caller can validate signatures now.
-      expect(await dao.connect(caller).isValidSignature(hash, goodSignature)).to
-        .not.be.reverted;
+      expect(await dao.connect(caller).isValidSignature(hash, signature)).to.not
+        .be.reverted;
 
-      // Because the caller is allowed unconditionally, the validation is always true.
+      // Because the caller is allowed unconditionally, the signature is always valid.
       expect(
-        await dao.connect(caller).isValidSignature(hash, goodSignature)
+        await dao.connect(caller).isValidSignature(hash, signature)
       ).to.equal(VALID_ERC1271_SIGNATURE);
+
+      // Because the other caller is not allowed, the signature is always invalid.
       expect(
-        await dao.connect(caller).isValidSignature(hash, badSignature)
-      ).to.equal(VALID_ERC1271_SIGNATURE);
+        await dao.connect(otherCaller).isValidSignature(hash, signature)
+      ).to.equal(INVALID_ERC1271_SIGNATURE);
     });
 
-    it('allows caller specific signature validation', async () => {
-      const caller = signers[0];
-      const signer = signers[1];
-
-      const message = 'Hi!';
-      const hash = ethers.utils.hashMessage(message);
-      const goodSignature = await signer.signMessage(message);
-      const badSignature = await signer.signMessage('Bye!');
-      expect(badSignature).to.not.equal(goodSignature);
-
+    it('allows caller-specific signature validation conditions', async () => {
       // Try to call with caller but caller has no permission
-      expect(await dao.connect(caller).isValidSignature(hash, goodSignature))
+      expect(await dao.connect(caller).isValidSignature(hash, signature))
         .to.be.revertedWithCustomError(dao, 'Unauthorized')
         .withArgs();
 
-      // Grant the permission to validate signatures to the caller unconditionally
-      await dao.grant(
+      // Deploy a mock condition
+      const mockCondition = await new PermissionConditionMock__factory(
+        caller
+      ).deploy();
+
+      // Grant the permission to validate signatures to the caller
+      await dao.grantWithCondition(
         dao.address,
         caller.address,
-        PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID
+        PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID,
+        mockCondition.address
       );
 
       // The caller can validate signatures now.
-      expect(await dao.connect(caller).isValidSignature(hash, goodSignature)).to
-        .not.be.reverted;
+      expect(await dao.connect(caller).isValidSignature(hash, signature)).to.not
+        .be.reverted;
 
-      // Because the caller is allowed unconditionally, the signature is always valid.
-      expect(
-        await dao.connect(caller).isValidSignature(hash, goodSignature)
-      ).to.equal(VALID_ERC1271_SIGNATURE);
+      // Check that the mock condition will answer true.
+      expect(await mockCondition.answer()).to.be.true;
+
+      // Check that the signature is invalid in this case.
       expect(
-        await dao.connect(caller).isValidSignature(hash, badSignature)
+        await dao.connect(caller).isValidSignature(hash, signature)
       ).to.equal(VALID_ERC1271_SIGNATURE);
-    });
 
-    it('allows generic signature validation', async () => {
-      const caller = signers[0];
-      const signer = signers[1];
-      const other = signers[3];
+      // Set the mock condition to answer false.
+      await mockCondition.setAnswer(false);
 
-      const message = 'Hi!';
-      const hash = ethers.utils.hashMessage(message);
-      const goodSignature = await signer.signMessage(message);
-      const badSignature = await signer.signMessage('Bye!');
-      expect(badSignature).to.not.equal(goodSignature);
+      // Check that the mock condition will answer false.
+      expect(await mockCondition.answer()).to.be.false;
 
-      // Try to call with caller but caller has no permission
-      expect(await dao.connect(caller).isValidSignature(hash, goodSignature))
-        .to.be.revertedWithCustomError(dao, 'Unauthorized')
-        .withArgs();
+      // Check that the signature is invalid in this case.
+      expect(
+        await dao.connect(caller).isValidSignature(hash, signature)
+      ).to.equal(INVALID_ERC1271_SIGNATURE);
+    });
 
+    it('allows generic signature validation by granting to ANY_ADDR', async () => {
+      // Deploy a mock condition
       const mockCondition = await new PermissionConditionMock__factory(
         caller
       ).deploy();
@@ -1337,25 +1321,34 @@ describe('DAO', function () {
         mockCondition.address
       );
 
+      // Check that the mock condition will answer true.
+      expect(await mockCondition.answer()).to.be.true;
+
       // Any contract can validate signatures now.
-      expect(await dao.connect(caller).isValidSignature(hash, goodSignature)).to
-        .not.be.reverted;
-      expect(await dao.connect(other).isValidSignature(hash, goodSignature)).to
-        .not.be.reverted;
+      expect(await dao.connect(caller).isValidSignature(hash, signature)).to.not
+        .be.reverted;
+      expect(await dao.connect(otherCaller).isValidSignature(hash, signature))
+        .to.not.be.reverted;
+
+      // Set the mock condition to answer false.
+      await mockCondition.setAnswer(false);
 
-      // Because we use a mock condition always returning true, the signature is always valid.
+      // Check that the mock condition will answer false.
+      expect(await mockCondition.answer()).to.be.false;
+
+      // Check that the signature is invalid in this case.
       expect(
-        await dao.connect(caller).isValidSignature(hash, goodSignature)
-      ).to.equal(VALID_ERC1271_SIGNATURE);
+        await dao.connect(caller).isValidSignature(hash, signature)
+      ).to.equal(INVALID_ERC1271_SIGNATURE);
       expect(
-        await dao.connect(caller).isValidSignature(hash, badSignature)
-      ).to.equal(VALID_ERC1271_SIGNATURE);
+        await dao.connect(otherCaller).isValidSignature(hash, signature)
+      ).to.equal(INVALID_ERC1271_SIGNATURE);
     });
 
     it('should revert if `setSignatureValidator` is called', async () => {
       await expect(
         dao
-          .connect(signers[2])
+          .connect(caller)
           .setSignatureValidator(ethers.Wallet.createRandom().address)
       ).to.be.revertedWithCustomError(dao, 'FunctionRemoved');
     });
diff --git a/packages/contracts/test/core/permission/permission-manager.ts b/packages/contracts/test/core/permission/permission-manager.ts
index 9824ec0ea..2034a8a9e 100644
--- a/packages/contracts/test/core/permission/permission-manager.ts
+++ b/packages/contracts/test/core/permission/permission-manager.ts
@@ -816,7 +816,7 @@ describe('Core: PermissionManager', function () {
         )
       ).to.be.equal(true);
 
-      await permissionCondition.setWillPerform(false);
+      await permissionCondition.setAnswer(false);
       expect(
         await pm.callStatic.isGranted(
           pm.address,

From 65816bcdf63e46c076085c0e544887bbb1c27e52 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Fri, 1 Sep 2023 16:06:41 +0200
Subject: [PATCH 19/38] docs: minor comment improvement

---
 packages/contracts/test/core/dao/dao.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index d61f53d63..4b6e41742 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -1324,7 +1324,7 @@ describe('DAO', function () {
       // Check that the mock condition will answer true.
       expect(await mockCondition.answer()).to.be.true;
 
-      // Any contract can validate signatures now.
+      // Any caller can validate signatures using this condition now.
       expect(await dao.connect(caller).isValidSignature(hash, signature)).to.not
         .be.reverted;
       expect(await dao.connect(otherCaller).isValidSignature(hash, signature))
@@ -1336,7 +1336,7 @@ describe('DAO', function () {
       // Check that the mock condition will answer false.
       expect(await mockCondition.answer()).to.be.false;
 
-      // Check that the signature is invalid in this case.
+      // Check that the signature is invalid in this case for every caller.
       expect(
         await dao.connect(caller).isValidSignature(hash, signature)
       ).to.equal(INVALID_ERC1271_SIGNATURE);

From 01941b8e01edd8883134f9bd237524a4420b28fd Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Fri, 1 Sep 2023 16:12:09 +0200
Subject: [PATCH 20/38] refactor: rename removed variable

---
 packages/contracts/src/core/dao/DAO.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/contracts/src/core/dao/DAO.sol b/packages/contracts/src/core/dao/DAO.sol
index 5048bf880..83ebea0de 100644
--- a/packages/contracts/src/core/dao/DAO.sol
+++ b/packages/contracts/src/core/dao/DAO.sol
@@ -72,7 +72,7 @@ contract DAO is
     /// @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;
+    address private __removed0;
 
     /// @notice The address of the trusted forwarder verifying meta transactions.
     /// @dev Added in v1.0.0.

From c287c05b75ac7fc4076838816a07edfe6beb9ac0 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Fri, 1 Sep 2023 16:13:17 +0200
Subject: [PATCH 21/38] docs: improve comment

---
 packages/contracts/test/core/dao/dao.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index 4b6e41742..22709a7fd 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -1313,7 +1313,7 @@ describe('DAO', function () {
         caller
       ).deploy();
 
-      // Grant the permission to validate signatures to the caller conditionally (granting it unconditionally is not possible in combination with `_who: ANY_ADDR`)
+      // Grant the permission to validate signatures to the ANY caller conditionally (granting it unconditionally is not possible in combination with `_who: ANY_ADDR`)
       await dao.grantWithCondition(
         dao.address,
         ANY_ADDR,

From 89817bd419c478c7130139366f6b24c53c7ca9a7 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Fri, 1 Sep 2023 16:20:05 +0200
Subject: [PATCH 22/38] test: check for correct value instead non-reverting
 call

---
 packages/contracts/test/core/dao/dao.ts | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index 22709a7fd..ceb650085 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -1325,10 +1325,12 @@ describe('DAO', function () {
       expect(await mockCondition.answer()).to.be.true;
 
       // Any caller can validate signatures using this condition now.
-      expect(await dao.connect(caller).isValidSignature(hash, signature)).to.not
-        .be.reverted;
-      expect(await dao.connect(otherCaller).isValidSignature(hash, signature))
-        .to.not.be.reverted;
+      expect(
+        await dao.connect(caller).isValidSignature(hash, signature)
+      ).to.equal(VALID_ERC1271_SIGNATURE);
+      expect(
+        await dao.connect(otherCaller).isValidSignature(hash, signature)
+      ).to.equal(VALID_ERC1271_SIGNATURE);
 
       // Set the mock condition to answer false.
       await mockCondition.setAnswer(false);

From cc7179e5dd8f060b14fb248f1a4415362a2f3ebe Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Fri, 1 Sep 2023 16:44:14 +0200
Subject: [PATCH 23/38] test: move factory creation into beforeEach

---
 packages/contracts/test/core/dao/dao.ts | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index ceb650085..f6105f09d 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -1225,11 +1225,15 @@ describe('DAO', function () {
     let hash: string;
     let signature: string;
 
+    let mockConditionFactory: PermissionConditionMock__factory;
+
     beforeEach(async () => {
       caller = signers[0];
       signer = signers[1];
       otherCaller = signers[2];
 
+      mockConditionFactory = new PermissionConditionMock__factory(caller);
+
       message = 'The message!';
       hash = ethers.utils.hashMessage(message);
       signature = await signer.signMessage(message);
@@ -1271,9 +1275,7 @@ describe('DAO', function () {
         .withArgs();
 
       // Deploy a mock condition
-      const mockCondition = await new PermissionConditionMock__factory(
-        caller
-      ).deploy();
+      const mockCondition = await mockConditionFactory.deploy();
 
       // Grant the permission to validate signatures to the caller
       await dao.grantWithCondition(
@@ -1309,9 +1311,7 @@ describe('DAO', function () {
 
     it('allows generic signature validation by granting to ANY_ADDR', async () => {
       // Deploy a mock condition
-      const mockCondition = await new PermissionConditionMock__factory(
-        caller
-      ).deploy();
+      const mockCondition = await mockConditionFactory.deploy();
 
       // Grant the permission to validate signatures to the ANY caller conditionally (granting it unconditionally is not possible in combination with `_who: ANY_ADDR`)
       await dao.grantWithCondition(

From 182fe93616c2ab670dec67e36c5da1c86a7b47d5 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Fri, 1 Sep 2023 17:07:52 +0200
Subject: [PATCH 24/38] test: a specific and a generic condition are granted at
 the same time

---
 packages/contracts/test/core/dao/dao.ts | 58 +++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index f6105f09d..36739ccbd 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -21,6 +21,7 @@ import {
   IEIP4824__factory,
   IProtocolVersion__factory,
   PermissionConditionMock__factory,
+  PermissionConditionMock,
 } from '../../../typechain';
 import {DAO__factory as DAO_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/core/dao/DAO.sol';
 import {DAO__factory as DAO_V1_3_0__factory} from '../../../typechain/@aragon/osx-v1.3.0-rc0.2/core/dao/DAO.sol';
@@ -54,6 +55,7 @@ import {
   IMPLICIT_INITIAL_PROTOCOL_VERSION,
 } from '../../test-utils/protocol-version';
 import {ANY_ADDR} from '../permission/permission-manager';
+import {defaultAbiCoder} from 'ethers/lib/utils';
 
 chai.use(smock.matchers);
 
@@ -1347,6 +1349,62 @@ describe('DAO', function () {
       ).to.equal(INVALID_ERC1271_SIGNATURE);
     });
 
+    context(
+      'A caller-specfic and a generic condition are both set',
+      async () => {
+        let specificMockCondition: PermissionConditionMock;
+        let genericMockCondition: PermissionConditionMock;
+
+        beforeEach(async () => {
+          // Setup the specfic condition for a specific caller
+          specificMockCondition = await mockConditionFactory.deploy();
+          await dao.grantWithCondition(
+            dao.address,
+            caller.address,
+            PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID,
+            specificMockCondition.address
+          );
+
+          // Setup the generic condition for ANY caller
+          genericMockCondition = await mockConditionFactory.deploy();
+          await dao.grantWithCondition(
+            dao.address,
+            ANY_ADDR,
+            PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID,
+            genericMockCondition.address
+          );
+        });
+
+        it('returns valid if both conditions are met', async () => {
+          expect(
+            await dao.connect(caller).isValidSignature(hash, signature)
+          ).to.equal(VALID_ERC1271_SIGNATURE);
+        });
+
+        it('returns valid if only the specific condition is met', async () => {
+          await genericMockCondition.setAnswer(false);
+          expect(
+            await dao.connect(caller).isValidSignature(hash, signature)
+          ).to.equal(VALID_ERC1271_SIGNATURE);
+        });
+
+        it('returns valid if only the generic condition is met', async () => {
+          await specificMockCondition.setAnswer(false);
+          expect(
+            await dao.connect(caller).isValidSignature(hash, signature)
+          ).to.equal(VALID_ERC1271_SIGNATURE);
+        });
+
+        it('returns invalid if both conditions are not met', async () => {
+          await specificMockCondition.setAnswer(false);
+          await genericMockCondition.setAnswer(false);
+          expect(
+            await dao.connect(caller).isValidSignature(hash, signature)
+          ).to.equal(INVALID_ERC1271_SIGNATURE);
+        });
+      }
+    );
+
     it('should revert if `setSignatureValidator` is called', async () => {
       await expect(
         dao

From 51abdb3c69402442160f66bebfb1b7b57ad1a860 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Mon, 4 Sep 2023 15:28:18 +0200
Subject: [PATCH 25/38] refactor: permission manager

refactor: simplifications
---
 packages/contracts/src/core/dao/DAO.sol       |   2 +-
 packages/contracts/src/core/dao/IDAO.sol      |   2 +-
 .../src/core/permission/PermissionManager.sol | 193 ++++++++++++------
 .../test/permission/PermissionManagerTest.sol |   2 +-
 packages/contracts/test/core/dao/dao.ts       |   3 +-
 5 files changed, 139 insertions(+), 63 deletions(-)

diff --git a/packages/contracts/src/core/dao/DAO.sol b/packages/contracts/src/core/dao/DAO.sol
index 83ebea0de..4bf3c58d7 100644
--- a/packages/contracts/src/core/dao/DAO.sol
+++ b/packages/contracts/src/core/dao/DAO.sol
@@ -223,7 +223,7 @@ contract DAO is
         bytes32 _permissionId,
         bytes memory _data
     ) external view override returns (bool) {
-        return isGranted(_where, _who, _permissionId, _data);
+        return isGranted({_where: _where, _who: _who, _permissionId: _permissionId, _data: _data});
     }
 
     /// @inheritdoc IDAO
diff --git a/packages/contracts/src/core/dao/IDAO.sol b/packages/contracts/src/core/dao/IDAO.sol
index 5b6c0d233..3084d9e6c 100644
--- a/packages/contracts/src/core/dao/IDAO.sol
+++ b/packages/contracts/src/core/dao/IDAO.sol
@@ -115,7 +115,7 @@ interface IDAO {
     /// @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.
+    /// @return Returns the `bytes4` magic value `0x1626ba7e` if the signature is valid and `0xffffffff` if not.
     function isValidSignature(bytes32 _hash, bytes memory _signature) external returns (bytes4);
 
     /// @notice Registers an ERC standard having a callback by registering its [ERC-165](https://eips.ethereum.org/EIPS/eip-165) interface ID and callback function signature.
diff --git a/packages/contracts/src/core/permission/PermissionManager.sol b/packages/contracts/src/core/permission/PermissionManager.sol
index 57236fd61..8de7cf2f4 100644
--- a/packages/contracts/src/core/permission/PermissionManager.sol
+++ b/packages/contracts/src/core/permission/PermissionManager.sol
@@ -105,7 +105,7 @@ abstract contract PermissionManager is Initializable {
     /// @dev The initial owner is granted the `ROOT_PERMISSION_ID` permission.
     /// @param _initialOwner The initial owner of the permission manager.
     function __PermissionManager_init(address _initialOwner) internal onlyInitializing {
-        _initializePermissionManager(_initialOwner);
+        _initializePermissionManager({_initialOwner: _initialOwner});
     }
 
     /// @notice Grants permission to an address to call methods in a contract guarded by an auth modifier with the specified permission identifier.
@@ -119,7 +119,7 @@ abstract contract PermissionManager is Initializable {
         address _who,
         bytes32 _permissionId
     ) external virtual auth(ROOT_PERMISSION_ID) {
-        _grant(_where, _who, _permissionId);
+        _grant({_where: _where, _who: _who, _permissionId: _permissionId});
     }
 
     /// @notice Grants permission to an address to call methods in a target contract guarded by an auth modifier with the specified permission identifier if the referenced condition permits it.
@@ -135,7 +135,12 @@ abstract contract PermissionManager is Initializable {
         bytes32 _permissionId,
         IPermissionCondition _condition
     ) external virtual auth(ROOT_PERMISSION_ID) {
-        _grantWithCondition(_where, _who, _permissionId, _condition);
+        _grantWithCondition({
+            _where: _where,
+            _who: _who,
+            _permissionId: _permissionId,
+            _condition: _condition
+        });
     }
 
     /// @notice Revokes permission from an address to call methods in a target contract guarded by an auth modifier with the specified permission identifier.
@@ -149,7 +154,7 @@ abstract contract PermissionManager is Initializable {
         address _who,
         bytes32 _permissionId
     ) external virtual auth(ROOT_PERMISSION_ID) {
-        _revoke(_where, _who, _permissionId);
+        _revoke({_where: _where, _who: _who, _permissionId: _permissionId});
     }
 
     /// @notice Applies an array of permission operations on a single target contracts `_where`.
@@ -163,9 +168,9 @@ abstract contract PermissionManager is Initializable {
             PermissionLib.SingleTargetPermission memory item = items[i];
 
             if (item.operation == PermissionLib.Operation.Grant) {
-                _grant(_where, item.who, item.permissionId);
+                _grant({_where: _where, _who: item.who, _permissionId: item.permissionId});
             } else if (item.operation == PermissionLib.Operation.Revoke) {
-                _revoke(_where, item.who, item.permissionId);
+                _revoke({_where: _where, _who: item.who, _permissionId: item.permissionId});
             } else if (item.operation == PermissionLib.Operation.GrantWithCondition) {
                 revert GrantWithConditionNotSupported();
             }
@@ -185,16 +190,16 @@ abstract contract PermissionManager is Initializable {
             PermissionLib.MultiTargetPermission memory item = _items[i];
 
             if (item.operation == PermissionLib.Operation.Grant) {
-                _grant(item.where, item.who, item.permissionId);
+                _grant({_where: item.where, _who: item.who, _permissionId: item.permissionId});
             } else if (item.operation == PermissionLib.Operation.Revoke) {
-                _revoke(item.where, item.who, item.permissionId);
+                _revoke({_where: item.where, _who: item.who, _permissionId: item.permissionId});
             } else if (item.operation == PermissionLib.Operation.GrantWithCondition) {
-                _grantWithCondition(
-                    item.where,
-                    item.who,
-                    item.permissionId,
-                    IPermissionCondition(item.condition)
-                );
+                _grantWithCondition({
+                    _where: item.where,
+                    _who: item.who,
+                    _permissionId: item.permissionId,
+                    _condition: IPermissionCondition(item.condition)
+                });
             }
 
             unchecked {
@@ -215,16 +220,96 @@ abstract contract PermissionManager is Initializable {
         bytes32 _permissionId,
         bytes memory _data
     ) public view virtual returns (bool) {
-        return
-            _isGranted(_where, _who, _permissionId, _data) || // check if `_who` has permission for `_permissionId` on `_where`
-            _isGranted(_where, ANY_ADDR, _permissionId, _data) || // check if anyone has permission for `_permissionId` on `_where`
-            _isGranted(ANY_ADDR, _who, _permissionId, _data); // check if `_who` has permission for `_permissionI` on any contract
+        // Specific caller and target
+        {
+            address specificPermissionOrCondition = permissionsHashed[
+                permissionHash({_where: _where, _who: _who, _permissionId: _permissionId})
+            ];
+
+            if (specificPermissionOrCondition == ALLOW_FLAG) return true;
+
+            if (specificPermissionOrCondition != UNSET_FLAG) {
+                return
+                    _checkCondition({
+                        _condition: specificPermissionOrCondition,
+                        _where: _where,
+                        _who: _who,
+                        _permissionId: _permissionId,
+                        _data: _data
+                    });
+            }
+        }
+
+        // Generic caller `_who: ANY_ADDR`  (only possible by granting with `grantWithCondition`)
+        {
+            address genericCallerCondition = permissionsHashed[
+                permissionHash({_where: _where, _who: ANY_ADDR, _permissionId: _permissionId})
+            ];
+
+            if (genericCallerCondition != UNSET_FLAG) {
+                return
+                    _checkCondition({
+                        _condition: genericCallerCondition,
+                        _where: _where,
+                        _who: _who,
+                        _permissionId: _permissionId,
+                        _data: _data
+                    });
+            }
+        }
+
+        // Generic target `_where: ANY_ADDR` (only possible by granting with `grantWithCondition`)
+        {
+            address genericTargetCondition = permissionsHashed[
+                permissionHash({_where: ANY_ADDR, _who: _who, _permissionId: _permissionId})
+            ];
+            if (genericTargetCondition != UNSET_FLAG) {
+                return
+                    _checkCondition({
+                        _condition: genericTargetCondition,
+                        _where: _where,
+                        _who: _who,
+                        _permissionId: _permissionId,
+                        _data: _data
+                    });
+            }
+        }
+
+        return false;
+    }
+
+    /// @notice Checks an condition contract by doing an external call.
+    /// @param _condition The condition contract that is asked.
+    /// @param _where The address of the target contract for which `_who` receives permission.
+    /// @param _who The address (EOA or contract) owning the permission.
+    /// @param _permissionId The permission identifier.
+    /// @param _data The optional data passed to the `PermissionCondition` registered.
+    /// @return Returns true if `_who` has the permissions on the contract via the specified permissionId identifier.
+    function _checkCondition(
+        address _condition,
+        address _where,
+        address _who,
+        bytes32 _permissionId,
+        bytes memory _data
+    ) internal view virtual returns (bool) {
+        // try-catch to skip failures
+        try
+            IPermissionCondition(_condition).isGranted({
+                _where: _where,
+                _who: _who,
+                _permissionId: _permissionId,
+                _data: _data
+            })
+        returns (bool result) {
+            if (result) return true;
+        } catch {}
+        return false;
     }
 
     /// @notice Grants the `ROOT_PERMISSION_ID` permission to the initial owner during initialization of the permission manager.
     /// @param _initialOwner The initial owner of the permission manager.
     function _initializePermissionManager(address _initialOwner) internal {
-        _grant(address(this), _initialOwner, ROOT_PERMISSION_ID);
+        _grant({_where: address(this), _who: _initialOwner, _permissionId: ROOT_PERMISSION_ID});
     }
 
     /// @notice This method is used in the external `grant` method of the permission manager.
@@ -237,7 +322,11 @@ abstract contract PermissionManager is Initializable {
             revert PermissionsForAnyAddressDisallowed();
         }
 
-        bytes32 permHash = permissionHash(_where, _who, _permissionId);
+        bytes32 permHash = permissionHash({
+            _where: _where,
+            _who: _who,
+            _permissionId: _permissionId
+        });
 
         address currentFlag = permissionsHashed[permHash];
 
@@ -245,7 +334,13 @@ abstract contract PermissionManager is Initializable {
         if (currentFlag == UNSET_FLAG) {
             permissionsHashed[permHash] = ALLOW_FLAG;
 
-            emit Granted(_permissionId, msg.sender, _where, _who, ALLOW_FLAG);
+            emit Granted({
+                permissionId: _permissionId,
+                here: msg.sender,
+                where: _where,
+                who: _who,
+                condition: ALLOW_FLAG
+            });
         }
     }
 
@@ -288,7 +383,11 @@ abstract contract PermissionManager is Initializable {
             }
         }
 
-        bytes32 permHash = permissionHash(_where, _who, _permissionId);
+        bytes32 permHash = permissionHash({
+            _where: _where,
+            _who: _who,
+            _permissionId: _permissionId
+        });
 
         address currentCondition = permissionsHashed[permHash];
 
@@ -296,7 +395,13 @@ abstract contract PermissionManager is Initializable {
         if (currentCondition == UNSET_FLAG) {
             permissionsHashed[permHash] = conditionAddr;
 
-            emit Granted(_permissionId, msg.sender, _where, _who, conditionAddr);
+            emit Granted({
+                permissionId: _permissionId,
+                here: msg.sender,
+                where: _where,
+                who: _who,
+                condition: conditionAddr
+            });
         } else if (currentCondition != conditionAddr) {
             // Revert if `permHash` is already granted, but uses a different condition.
             // If we don't revert, we either should:
@@ -319,48 +424,18 @@ abstract contract PermissionManager is Initializable {
     /// @param _permissionId The permission identifier.
     /// @dev Note, that revoking permissions with `_who` or `_where` equal to `ANY_ADDR` does not revoke other permissions with specific `_who` and `_where` addresses that might have been granted in parallel.
     function _revoke(address _where, address _who, bytes32 _permissionId) internal virtual {
-        bytes32 permHash = permissionHash(_where, _who, _permissionId);
+        bytes32 permHash = permissionHash({
+            _where: _where,
+            _who: _who,
+            _permissionId: _permissionId
+        });
         if (permissionsHashed[permHash] != UNSET_FLAG) {
             permissionsHashed[permHash] = UNSET_FLAG;
 
-            emit Revoked(_permissionId, msg.sender, _where, _who);
+            emit Revoked({permissionId: _permissionId, here: msg.sender, where: _where, who: _who});
         }
     }
 
-    /// @notice Checks if a caller is granted permissions on a target contract via a permission identifier and redirects the approval to a `PermissionCondition` if this was specified in the setup.
-    /// @param _where The address of the target contract for which `_who` receives permission.
-    /// @param _who The address (EOA or contract) owning the permission.
-    /// @param _permissionId The permission identifier.
-    /// @param _data The optional data passed to the `PermissionCondition` registered.
-    /// @return Returns true if `_who` has the permissions on the contract via the specified permissionId identifier.
-    function _isGranted(
-        address _where,
-        address _who,
-        bytes32 _permissionId,
-        bytes memory _data
-    ) internal view virtual returns (bool) {
-        address accessFlagOrCondition = permissionsHashed[
-            permissionHash(_where, _who, _permissionId)
-        ];
-
-        if (accessFlagOrCondition == UNSET_FLAG) return false;
-        if (accessFlagOrCondition == ALLOW_FLAG) return true;
-
-        // Since it's not a flag, assume it's a PermissionCondition and try-catch to skip failures
-        try
-            IPermissionCondition(accessFlagOrCondition).isGranted(
-                _where,
-                _who,
-                _permissionId,
-                _data
-            )
-        returns (bool allowed) {
-            if (allowed) return true;
-        } catch {}
-
-        return false;
-    }
-
     /// @notice A private function to be used to check permissions on the permission manager contract (`address(this)`) itself.
     /// @param _permissionId The permission identifier required to call the method this modifier is applied to.
     function _auth(bytes32 _permissionId) internal view virtual {
diff --git a/packages/contracts/src/test/permission/PermissionManagerTest.sol b/packages/contracts/src/test/permission/PermissionManagerTest.sol
index 029a27cc7..250b80910 100644
--- a/packages/contracts/src/test/permission/PermissionManagerTest.sol
+++ b/packages/contracts/src/test/permission/PermissionManagerTest.sol
@@ -41,7 +41,7 @@ contract PermissionManagerTest is PermissionManager {
         bytes32 _permissionId,
         bytes memory _data
     ) public view returns (bool) {
-        return _isGranted(_where, _who, _permissionId, _data);
+        return isGranted(_where, _who, _permissionId, _data);
     }
 
     function isPermissionRestrictedForAnyAddr(
diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index 36739ccbd..67facd66f 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -1330,6 +1330,7 @@ describe('DAO', function () {
       expect(
         await dao.connect(caller).isValidSignature(hash, signature)
       ).to.equal(VALID_ERC1271_SIGNATURE);
+
       expect(
         await dao.connect(otherCaller).isValidSignature(hash, signature)
       ).to.equal(VALID_ERC1271_SIGNATURE);
@@ -1392,7 +1393,7 @@ describe('DAO', function () {
           await specificMockCondition.setAnswer(false);
           expect(
             await dao.connect(caller).isValidSignature(hash, signature)
-          ).to.equal(VALID_ERC1271_SIGNATURE);
+          ).to.equal(INVALID_ERC1271_SIGNATURE);
         });
 
         it('returns invalid if both conditions are not met', async () => {

From 1e68b7db4ff53814b9ccab16acf58087465a77d1 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Tue, 19 Sep 2023 12:15:39 +0200
Subject: [PATCH 26/38] style: naming and comments

---
 .../src/core/permission/PermissionManager.sol | 34 ++++++++++++-------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/packages/contracts/src/core/permission/PermissionManager.sol b/packages/contracts/src/core/permission/PermissionManager.sol
index 8de7cf2f4..b34249f76 100644
--- a/packages/contracts/src/core/permission/PermissionManager.sol
+++ b/packages/contracts/src/core/permission/PermissionManager.sol
@@ -220,53 +220,61 @@ abstract contract PermissionManager is Initializable {
         bytes32 _permissionId,
         bytes memory _data
     ) public view virtual returns (bool) {
-        // Specific caller and target
+        // Specific caller (`_who`) and target (`_where`) permission check
         {
-            address specificPermissionOrCondition = permissionsHashed[
+            // This permission may have been granted directly via the `grant` function or with a condition via the `grantWithCondition` function.
+            address specificCallerTargetPermission = permissionsHashed[
                 permissionHash({_where: _where, _who: _who, _permissionId: _permissionId})
             ];
 
-            if (specificPermissionOrCondition == ALLOW_FLAG) return true;
+            // If it is directly granted, return true.
+            if (specificCallerTargetPermission == ALLOW_FLAG) return true;
 
-            if (specificPermissionOrCondition != UNSET_FLAG) {
+            // If it points to a condition address, check it.
+            if (specificCallerTargetPermission != UNSET_FLAG) {
                 return
                     _checkCondition({
-                        _condition: specificPermissionOrCondition,
+                        _condition: specificCallerTargetPermission,
                         _where: _where,
                         _who: _who,
                         _permissionId: _permissionId,
                         _data: _data
                     });
             }
+
+            // If this permission is not set, check if there is a generic caller condition.
         }
 
-        // Generic caller `_who: ANY_ADDR`  (only possible by granting with `grantWithCondition`)
+        // Generic caller (`_who: ANY_ADDR`) condition check
         {
-            address genericCallerCondition = permissionsHashed[
+            // This permission can only be granted in conjunction with a condition via the `grantWithCondition` function.
+            address genericCallerPermissionCondition = permissionsHashed[
                 permissionHash({_where: _where, _who: ANY_ADDR, _permissionId: _permissionId})
             ];
 
-            if (genericCallerCondition != UNSET_FLAG) {
+            if (genericCallerPermissionCondition != UNSET_FLAG) {
                 return
                     _checkCondition({
-                        _condition: genericCallerCondition,
+                        _condition: genericCallerPermissionCondition,
                         _where: _where,
                         _who: _who,
                         _permissionId: _permissionId,
                         _data: _data
                     });
             }
+            // If no generic caller condition is set, check if there is a generic target condition.
         }
 
-        // Generic target `_where: ANY_ADDR` (only possible by granting with `grantWithCondition`)
+        // Generic target (`_where: ANY_ADDR`) condition check
         {
-            address genericTargetCondition = permissionsHashed[
+            // This permission can only be granted in conjunction with a condition via the `grantWithCondition` function.
+            address genericTargetPermissionCondition = permissionsHashed[
                 permissionHash({_where: ANY_ADDR, _who: _who, _permissionId: _permissionId})
             ];
-            if (genericTargetCondition != UNSET_FLAG) {
+            if (genericTargetPermissionCondition != UNSET_FLAG) {
                 return
                     _checkCondition({
-                        _condition: genericTargetCondition,
+                        _condition: genericTargetPermissionCondition,
                         _where: _where,
                         _who: _who,
                         _permissionId: _permissionId,

From eee576b144a5e22382055171592b033c3af660cf Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Tue, 19 Sep 2023 14:38:39 +0200
Subject: [PATCH 27/38] test: added and refactored tests of isGranted

---
 .../core/permission/permission-manager.ts     | 302 ++++++++++++++++--
 .../contracts/test/test-utils/conditions.ts   |  13 -
 2 files changed, 282 insertions(+), 33 deletions(-)
 delete mode 100644 packages/contracts/test/test-utils/conditions.ts

diff --git a/packages/contracts/test/core/permission/permission-manager.ts b/packages/contracts/test/core/permission/permission-manager.ts
index 2034a8a9e..321a98523 100644
--- a/packages/contracts/test/core/permission/permission-manager.ts
+++ b/packages/contracts/test/core/permission/permission-manager.ts
@@ -9,7 +9,6 @@ import {
   PermissionConditionMock__factory,
   TestPlugin__factory,
 } from '../../../typechain';
-import {DeployTestPermissionCondition} from '../../test-utils/conditions';
 import {OZ_ERRORS} from '../../test-utils/error';
 import {Operation} from '../../../utils/types';
 
@@ -728,7 +727,7 @@ describe('Core: PermissionManager', function () {
   });
 
   describe('isGranted', () => {
-    it('should return true if the permission is granted to the user', async () => {
+    it('returns `true` if the permission is granted to the user', async () => {
       await pm.grant(pm.address, otherSigner.address, ADMIN_PERMISSION_ID);
       const isGranted = await pm.callStatic.isGranted(
         pm.address,
@@ -739,7 +738,7 @@ describe('Core: PermissionManager', function () {
       expect(isGranted).to.be.equal(true);
     });
 
-    it('should return false if the permissions is not granted to the user', async () => {
+    it('returns `false` if the permission is not granted to the user', async () => {
       const isGranted = await pm.callStatic.isGranted(
         pm.address,
         otherSigner.address,
@@ -749,35 +748,112 @@ describe('Core: PermissionManager', function () {
       expect(isGranted).to.be.equal(false);
     });
 
-    it('should return true for permissions granted to any address on a specific target contract using the `ANY_ADDR` flag', async () => {
-      const anyAddr = await pm.getAnyAddr();
-      const condition = await DeployTestPermissionCondition();
+    it('returns `true` if a condition is set for a specific caller and target answering `true`', async () => {
+      const condition = await new PermissionConditionMock__factory(
+        signers[0]
+      ).deploy();
       await pm.grantWithCondition(
         pm.address,
-        anyAddr,
+        ownerSigner.address,
         ADMIN_PERMISSION_ID,
         condition.address
       );
+      await condition.setAnswer(true);
+
+      expect(
+        await pm.isGranted(
+          pm.address,
+          ownerSigner.address,
+          ADMIN_PERMISSION_ID,
+          condition.address
+        )
+      ).to.be.true;
+    });
+
+    it('returns `true` if a condition is set for a generic caller answering `true`', async () => {
+      const condition = await new PermissionConditionMock__factory(
+        signers[0]
+      ).deploy();
       await pm.grantWithCondition(
-        anyAddr,
-        pm.address,
-        ADMIN_PERMISSION_ID,
-        condition.address
-      );
-      const isGranted_1 = await pm.callStatic.isGranted(
         pm.address,
-        anyAddr,
+        ANY_ADDR,
         ADMIN_PERMISSION_ID,
         condition.address
       );
-      const isGranted_2 = await pm.callStatic.isGranted(
-        pm.address,
-        anyAddr,
+      await condition.setAnswer(true);
+
+      // Check `ownerSigner.address` as a caller `_who`
+      expect(
+        await pm.isGranted(
+          pm.address,
+          ownerSigner.address,
+          ADMIN_PERMISSION_ID,
+          condition.address
+        )
+      ).to.be.true;
+
+      // Check `otherSigner.address` as a caller `_who`
+      expect(
+        await pm.isGranted(
+          pm.address,
+          otherSigner.address,
+          ADMIN_PERMISSION_ID,
+          condition.address
+        )
+      ).to.be.true;
+
+      // Check that `false` is returned if `address(0)` is the target `_where`.
+      expect(
+        await pm.isGranted(
+          ethers.constants.AddressZero,
+          ownerSigner.address,
+          ADMIN_PERMISSION_ID,
+          condition.address
+        )
+      ).to.be.false;
+    });
+
+    it('returns `true` if a condition is set for a generic target answering `true`', async () => {
+      const condition = await new PermissionConditionMock__factory(
+        signers[0]
+      ).deploy();
+      await pm.grantWithCondition(
+        ANY_ADDR,
+        ownerSigner.address,
         ADMIN_PERMISSION_ID,
         condition.address
       );
-      expect(isGranted_1).to.be.equal(true);
-      expect(isGranted_2).to.be.equal(true);
+      await condition.setAnswer(true);
+
+      // Check `pm.address` as a target `_where`
+      expect(
+        await pm.isGranted(
+          pm.address,
+          ownerSigner.address,
+          ADMIN_PERMISSION_ID,
+          condition.address
+        )
+      ).to.be.true;
+
+      // Check `address(0)` as a target `_where`
+      expect(
+        await pm.isGranted(
+          ethers.constants.AddressZero,
+          ownerSigner.address,
+          ADMIN_PERMISSION_ID,
+          condition.address
+        )
+      ).to.be.true;
+
+      // Check that `false` is returned if `otherSigner is the caller `_who`.
+      expect(
+        await pm.isGranted(
+          ethers.constants.AddressZero,
+          otherSigner.address,
+          ADMIN_PERMISSION_ID,
+          condition.address
+        )
+      ).to.be.false;
     });
 
     it('should be callable by anyone', async () => {
@@ -791,13 +867,199 @@ describe('Core: PermissionManager', function () {
         );
       expect(isGranted).to.be.equal(false);
     });
+
+    it('does not fall back to a generic caller or target condition if a specific condition is set already answering `false`', async () => {
+      const specificCondition = await new PermissionConditionMock__factory(
+        signers[0]
+      ).deploy();
+      const genericCallerCondition = await new PermissionConditionMock__factory(
+        signers[0]
+      ).deploy();
+      const genericTargetCondition = await new PermissionConditionMock__factory(
+        signers[0]
+      ).deploy();
+
+      // Grant with a specific condition that will answer false
+      await pm.grantWithCondition(
+        pm.address,
+        ownerSigner.address,
+        ADMIN_PERMISSION_ID,
+        specificCondition.address
+      );
+      await specificCondition.setAnswer(false);
+
+      // Grant with a generic caller condition that will answer true
+      await pm.grantWithCondition(
+        pm.address,
+        ANY_ADDR,
+        ADMIN_PERMISSION_ID,
+        genericCallerCondition.address
+      );
+      await genericCallerCondition.setAnswer(true);
+
+      // Grant with a generic target condition that will answer true
+      await pm.grantWithCondition(
+        ANY_ADDR,
+        ownerSigner.address,
+        ADMIN_PERMISSION_ID,
+        genericTargetCondition.address
+      );
+      await genericCallerCondition.setAnswer(true);
+
+      // Check that `isGranted` returns false for `ownerSigner` to whom the specific condition was granted.
+      expect(
+        await pm.isGranted(
+          pm.address,
+          ownerSigner.address,
+          ADMIN_PERMISSION_ID,
+          genericTargetCondition.address
+        )
+      ).to.be.false;
+
+      // Check that `ownerSigner` is still granted access to other contracts (e.g., `address(0)`) through the `genericTargetCondition` condition.
+      expect(
+        await pm.isGranted(
+          ethers.constants.AddressZero,
+          ownerSigner.address,
+          ADMIN_PERMISSION_ID,
+          genericTargetCondition.address
+        )
+      ).to.be.true;
+    });
+
+    it('does not fall back to a generic target condition if a generic caller condition is set already answering `false`', async () => {
+      const genericCallerCondition = await new PermissionConditionMock__factory(
+        signers[0]
+      ).deploy();
+      const genericTargetCondition = await new PermissionConditionMock__factory(
+        signers[0]
+      ).deploy();
+
+      // Grant with a generic caller condition that will answer false.
+      await pm.grantWithCondition(
+        pm.address,
+        ANY_ADDR,
+        ADMIN_PERMISSION_ID,
+        genericCallerCondition.address
+      );
+      await genericCallerCondition.setAnswer(false);
+
+      // Grant with a generic target condition that will answer true.
+      await pm.grantWithCondition(
+        ANY_ADDR,
+        ownerSigner.address,
+        ADMIN_PERMISSION_ID,
+        genericTargetCondition.address
+      );
+      await genericTargetCondition.setAnswer(true);
+
+      // Check that `isGranted` returns false for `ANY_ADDR` (here, we check only two addresses, `ownerSigner` and `otherSigner`).
+      expect(
+        await pm.isGranted(
+          pm.address,
+          ownerSigner.address,
+          ADMIN_PERMISSION_ID,
+          genericTargetCondition.address
+        )
+      ).to.be.false;
+      //
+      expect(
+        await pm.isGranted(
+          pm.address,
+          otherSigner.address,
+          ADMIN_PERMISSION_ID,
+          genericTargetCondition.address
+        )
+      ).to.be.false;
+
+      // Check that `ownerSigner` is granted access to other contracts (e.g., `address(0)`) via the `genericTargetCondition` condition.
+      expect(
+        await pm.isGranted(
+          ethers.constants.AddressZero,
+          ownerSigner.address,
+          ADMIN_PERMISSION_ID,
+          genericTargetCondition.address
+        )
+      ).to.be.true;
+
+      // Check that `otherSigner` is not granted access to other contracts (e.g., `address(0)`) via the `genericTargetCondition` condition.
+      expect(
+        await pm.isGranted(
+          ethers.constants.AddressZero,
+          otherSigner.address,
+          ADMIN_PERMISSION_ID,
+          genericTargetCondition.address
+        )
+      ).to.be.false;
+    });
+
+    it('does not fall back to a generic caller or target condition if a specific condition is set already answering `false`', async () => {
+      const specificCondition = await new PermissionConditionMock__factory(
+        signers[0]
+      ).deploy();
+      const genericCallerCondition = await new PermissionConditionMock__factory(
+        signers[0]
+      ).deploy();
+      const genericTargetCondition = await new PermissionConditionMock__factory(
+        signers[0]
+      ).deploy();
+
+      // Grant with a specific condition that will answer false
+      await pm.grantWithCondition(
+        pm.address,
+        ownerSigner.address,
+        ADMIN_PERMISSION_ID,
+        specificCondition.address
+      );
+      await specificCondition.setAnswer(false);
+
+      // Grant with a generic caller condition that will answer true
+      await pm.grantWithCondition(
+        pm.address,
+        ANY_ADDR,
+        ADMIN_PERMISSION_ID,
+        genericCallerCondition.address
+      );
+      await genericCallerCondition.setAnswer(true);
+
+      // Grant with a generic target condition that will answer true
+      await pm.grantWithCondition(
+        ANY_ADDR,
+        ownerSigner.address,
+        ADMIN_PERMISSION_ID,
+        genericTargetCondition.address
+      );
+      await genericCallerCondition.setAnswer(true);
+
+      // Check that `isGranted` returns false for `ownerSigner` to whom the specific condition was granted.
+      expect(
+        await pm.isGranted(
+          pm.address,
+          ownerSigner.address,
+          ADMIN_PERMISSION_ID,
+          genericTargetCondition.address
+        )
+      ).to.be.false;
+
+      // Check that `ownerSigner` is still granted access to other contracts (e.g., `address(0)`) through the `genericTargetCondition` condition.
+      expect(
+        await pm.isGranted(
+          ethers.constants.AddressZero,
+          ownerSigner.address,
+          ADMIN_PERMISSION_ID,
+          genericTargetCondition.address
+        )
+      ).to.be.true;
+    });
   });
 
   describe('_hasPermission', () => {
     let permissionCondition: PermissionConditionMock;
 
     beforeEach(async () => {
-      permissionCondition = await DeployTestPermissionCondition();
+      permissionCondition = await new PermissionConditionMock__factory(
+        ownerSigner
+      ).deploy();
     });
 
     it('should call IPermissionCondition.isGranted', async () => {
diff --git a/packages/contracts/test/test-utils/conditions.ts b/packages/contracts/test/test-utils/conditions.ts
deleted file mode 100644
index 17cc58971..000000000
--- a/packages/contracts/test/test-utils/conditions.ts
+++ /dev/null
@@ -1,13 +0,0 @@
-import {ethers} from 'hardhat';
-
-import {
-  PermissionConditionMock,
-  PermissionConditionMock__factory,
-} from '../../typechain';
-
-export async function DeployTestPermissionCondition(): Promise<PermissionConditionMock> {
-  const signers = await ethers.getSigners();
-  const aclConditionFactory = new PermissionConditionMock__factory(signers[0]);
-  const permissionCondition = await aclConditionFactory.deploy();
-  return permissionCondition;
-}

From 15c39c5819e9422da3df78b2c644d85a869cdc1d Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Tue, 19 Sep 2023 15:06:44 +0200
Subject: [PATCH 28/38] docs: improve NatSpec

---
 .../src/core/permission/PermissionManager.sol       | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/packages/contracts/src/core/permission/PermissionManager.sol b/packages/contracts/src/core/permission/PermissionManager.sol
index b34249f76..6a0bce7e7 100644
--- a/packages/contracts/src/core/permission/PermissionManager.sol
+++ b/packages/contracts/src/core/permission/PermissionManager.sol
@@ -208,7 +208,7 @@ abstract contract PermissionManager is Initializable {
         }
     }
 
-    /// @notice Checks if an address has permission on a contract via a permission identifier and considers if `ANY_ADDRESS` was used in the granting process.
+    /// @notice Checks if an address has permission on a contract via a permission identifier and relays the answer to a condition contract if this was declared during the the granting process.
     /// @param _where The address of the target contract for which `_who` receives permission.
     /// @param _who The address (EOA or contract) for which the permission is checked.
     /// @param _permissionId The permission identifier.
@@ -230,7 +230,7 @@ abstract contract PermissionManager is Initializable {
             // If it is directly granted, return true.
             if (specificCallerTargetPermission == ALLOW_FLAG) return true;
 
-            // If it points to a condition address, check it.
+            // If it points to a condition address, check the condition.
             if (specificCallerTargetPermission != UNSET_FLAG) {
                 return
                     _checkCondition({
@@ -286,13 +286,14 @@ abstract contract PermissionManager is Initializable {
         return false;
     }
 
-    /// @notice Checks an condition contract by doing an external call.
-    /// @param _condition The condition contract that is asked.
+    /// @notice Checks a condition contract by doing an external call via try/catch.
+    /// @param _condition The condition contract that is called.
     /// @param _where The address of the target contract for which `_who` receives permission.
     /// @param _who The address (EOA or contract) owning the permission.
     /// @param _permissionId The permission identifier.
     /// @param _data The optional data passed to the `PermissionCondition` registered.
-    /// @return Returns true if `_who` has the permissions on the contract via the specified permissionId identifier.
+    /// @return Returns `true` if `_who` has the permissions on the contract via the specified permissionId identifier.
+    /// @dev If the external call fails, we return `false`.
     function _checkCondition(
         address _condition,
         address _where,
@@ -300,7 +301,7 @@ abstract contract PermissionManager is Initializable {
         bytes32 _permissionId,
         bytes memory _data
     ) internal view virtual returns (bool) {
-        // try-catch to skip failures
+        // Try-catch to skip failures
         try
             IPermissionCondition(_condition).isGranted({
                 _where: _where,

From 0ec32c2c9e9c8bba352209ae73d081237edc53e3 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Tue, 19 Sep 2023 15:06:58 +0200
Subject: [PATCH 29/38] chore: maintained changelog

---
 packages/contracts/CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/packages/contracts/CHANGELOG.md b/packages/contracts/CHANGELOG.md
index 4fe77298d..7183d9691 100644
--- a/packages/contracts/CHANGELOG.md
+++ b/packages/contracts/CHANGELOG.md
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- 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`.
 - Use the DAOs permission manager functionality to validate signatures.
 

From c1b5fa34a7346e98441f6ef37a026f689111b9d9 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Tue, 19 Sep 2023 15:20:11 +0200
Subject: [PATCH 30/38] docs: updated docs

---
 .../docs/osx/01-how-it-works/01-core/02-permissions/index.md | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md b/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md
index 2090809fc..88794d649 100644
--- a/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md
+++ b/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md
@@ -133,6 +133,11 @@ Granting a permission with `_where: ANY_ADDR` to a condition has the effect that
 Imagine, for example, that many instances of the `Service` contract exist, and a user should have the permission to use all of them. By granting the `USE_PERMISSION_ID` with `_where: ANY_ADDR`, to some user `_who: userAddr`, the user has access to all of them. If this should not be possible anymore, you can later revoke the permission.
 
 However, some restrictions apply. For security reasons, Aragon OSx does not allow you to use both, `_where: ANY_ADDR` and `_who: ANY_ADDR` in the same permission. Furthermore, the permission IDs of [permissions native to the `DAO` Contract](#permissions-native-to-the-dao-contract) cannot be used.
+Moreover, if a condition already answered `false`, we do not fall back to a more generic one. The checks will proceed in the following order
+
+1. Condition with specific `_who` and specific `where`.
+2. Condition with generic `_who: ANY_ADDR` and specific `_where`.
+3. Condition with specific `_where` and generic `_who: ANY_ADDR`.
 
 ### Permissions Native to the `DAO` Contract
 

From ea9a81d7c55c4a24ac2f2f780b965a97e2cece27 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Tue, 19 Sep 2023 15:40:52 +0200
Subject: [PATCH 31/38] docs: improved condition order explanation

---
 .../docs/osx/01-how-it-works/01-core/02-permissions/index.md    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md b/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md
index 88794d649..96ca1d1ae 100644
--- a/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md
+++ b/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md
@@ -133,7 +133,7 @@ Granting a permission with `_where: ANY_ADDR` to a condition has the effect that
 Imagine, for example, that many instances of the `Service` contract exist, and a user should have the permission to use all of them. By granting the `USE_PERMISSION_ID` with `_where: ANY_ADDR`, to some user `_who: userAddr`, the user has access to all of them. If this should not be possible anymore, you can later revoke the permission.
 
 However, some restrictions apply. For security reasons, Aragon OSx does not allow you to use both, `_where: ANY_ADDR` and `_who: ANY_ADDR` in the same permission. Furthermore, the permission IDs of [permissions native to the `DAO` Contract](#permissions-native-to-the-dao-contract) cannot be used.
-Moreover, if a condition already answered `false`, we do not fall back to a more generic one. The checks will proceed in the following order
+Moreover, if a condition is set, we return its `isGranted` result and do not fall back to a more generic one. The condition checks occur in the following order
 
 1. Condition with specific `_who` and specific `where`.
 2. Condition with generic `_who: ANY_ADDR` and specific `_where`.

From ab2a4203454ba16ac0d10285723b51055d619afe Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Tue, 19 Sep 2023 16:12:12 +0200
Subject: [PATCH 32/38] docs: improved natspec

---
 .../contracts/src/core/permission/PermissionManager.sol  | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/packages/contracts/src/core/permission/PermissionManager.sol b/packages/contracts/src/core/permission/PermissionManager.sol
index 6a0bce7e7..58fd1baa5 100644
--- a/packages/contracts/src/core/permission/PermissionManager.sol
+++ b/packages/contracts/src/core/permission/PermissionManager.sol
@@ -208,11 +208,11 @@ abstract contract PermissionManager is Initializable {
         }
     }
 
-    /// @notice Checks if an address has permission on a contract via a permission identifier and relays the answer to a condition contract if this was declared during the the granting process.
+    /// @notice Checks if caller address has permission on target contract via a permission identifier and relays the answer to a condition contract if this was declared during the the granting process.
     /// @param _where The address of the target contract for which `_who` receives permission.
     /// @param _who The address (EOA or contract) for which the permission is checked.
     /// @param _permissionId The permission identifier.
-    /// @param _data The optional data passed to the `PermissionCondition` registered.
+    /// @param _data Optional data to be passed to a referenced `PermissionCondition`.
     /// @return Returns true if `_who` has the permissions on the target contract via the specified permission identifier.
     function isGranted(
         address _where,
@@ -286,13 +286,14 @@ abstract contract PermissionManager is Initializable {
         return false;
     }
 
+    /// @notice Relays the question if caller address has permission on target contract via a permission identifier to a condition contract.
     /// @notice Checks a condition contract by doing an external call via try/catch.
     /// @param _condition The condition contract that is called.
     /// @param _where The address of the target contract for which `_who` receives permission.
     /// @param _who The address (EOA or contract) owning the permission.
     /// @param _permissionId The permission identifier.
-    /// @param _data The optional data passed to the `PermissionCondition` registered.
-    /// @return Returns `true` if `_who` has the permissions on the contract via the specified permissionId identifier.
+    /// @param _data Optional data to be passed to a referenced `PermissionCondition`.
+    /// @return Returns `true` if a caller (`_who`) has the permissions on the contract (`_where`) via the specified permission identifier.
     /// @dev If the external call fails, we return `false`.
     function _checkCondition(
         address _condition,

From 60f8d7210f0b9c48d33dbbe658f7a36619d589d7 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Thu, 21 Sep 2023 10:27:13 +0200
Subject: [PATCH 33/38] refactor: naming, comments, and bracketing

---
 .../src/core/permission/PermissionManager.sol | 24 ++++++++++++-------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/packages/contracts/src/core/permission/PermissionManager.sol b/packages/contracts/src/core/permission/PermissionManager.sol
index 58fd1baa5..88496c11b 100644
--- a/packages/contracts/src/core/permission/PermissionManager.sol
+++ b/packages/contracts/src/core/permission/PermissionManager.sol
@@ -227,10 +227,10 @@ abstract contract PermissionManager is Initializable {
                 permissionHash({_where: _where, _who: _who, _permissionId: _permissionId})
             ];
 
-            // If it is directly granted, return true.
+            // If the permission was granted directly, return `true`.
             if (specificCallerTargetPermission == ALLOW_FLAG) return true;
 
-            // If it points to a condition address, check the condition.
+            // If the permission was granted with a condition, check the condition and return the result.
             if (specificCallerTargetPermission != UNSET_FLAG) {
                 return
                     _checkCondition({
@@ -248,14 +248,15 @@ abstract contract PermissionManager is Initializable {
         // Generic caller (`_who: ANY_ADDR`) condition check
         {
             // This permission can only be granted in conjunction with a condition via the `grantWithCondition` function.
-            address genericCallerPermissionCondition = permissionsHashed[
+            address genericCallerPermission = permissionsHashed[
                 permissionHash({_where: _where, _who: ANY_ADDR, _permissionId: _permissionId})
             ];
 
-            if (genericCallerPermissionCondition != UNSET_FLAG) {
+            // If the permission was granted with a condition, check the condition and return the result.
+            if (genericCallerPermission != UNSET_FLAG) {
                 return
                     _checkCondition({
-                        _condition: genericCallerPermissionCondition,
+                        _condition: genericCallerPermission,
                         _where: _where,
                         _who: _who,
                         _permissionId: _permissionId,
@@ -268,13 +269,15 @@ abstract contract PermissionManager is Initializable {
         // Generic target (`_where: ANY_ADDR`) condition check
         {
             // This permission can only be granted in conjunction with a condition via the `grantWithCondition` function.
-            address genericTargetPermissionCondition = permissionsHashed[
+            address genericTargetPermission = permissionsHashed[
                 permissionHash({_where: ANY_ADDR, _who: _who, _permissionId: _permissionId})
             ];
-            if (genericTargetPermissionCondition != UNSET_FLAG) {
+
+            // If the permission was granted with a condition, check the condition and return the result.
+            if (genericTargetPermission != UNSET_FLAG) {
                 return
                     _checkCondition({
-                        _condition: genericTargetPermissionCondition,
+                        _condition: genericTargetPermission,
                         _where: _where,
                         _who: _who,
                         _permissionId: _permissionId,
@@ -283,6 +286,7 @@ abstract contract PermissionManager is Initializable {
             }
         }
 
+        // No specific or generic permission (condition) applies so we return `false`.
         return false;
     }
 
@@ -311,7 +315,9 @@ abstract contract PermissionManager is Initializable {
                 _data: _data
             })
         returns (bool result) {
-            if (result) return true;
+            if (result) {
+                return true;
+            }
         } catch {}
         return false;
     }

From 2e9f55dbcb183aa975230b74f36bd71495877d62 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Thu, 21 Sep 2023 10:31:37 +0200
Subject: [PATCH 34/38] docs: improved comment

---
 .../contracts/src/core/permission/PermissionManager.sol    | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/packages/contracts/src/core/permission/PermissionManager.sol b/packages/contracts/src/core/permission/PermissionManager.sol
index 88496c11b..29b55f4fd 100644
--- a/packages/contracts/src/core/permission/PermissionManager.sol
+++ b/packages/contracts/src/core/permission/PermissionManager.sol
@@ -242,7 +242,7 @@ abstract contract PermissionManager is Initializable {
                     });
             }
 
-            // If this permission is not set, check if there is a generic caller condition.
+            // If this permission is not set, continue.
         }
 
         // Generic caller (`_who: ANY_ADDR`) condition check
@@ -263,7 +263,7 @@ abstract contract PermissionManager is Initializable {
                         _data: _data
                     });
             }
-            // If no generic caller condition is set, check if there is a generic target condition.
+            // If this permission is not set, continue.
         }
 
         // Generic target (`_where: ANY_ADDR`) condition check
@@ -284,9 +284,10 @@ abstract contract PermissionManager is Initializable {
                         _data: _data
                     });
             }
+            // If this permission is not set, continue.
         }
 
-        // No specific or generic permission (condition) applies so we return `false`.
+        // No specific or generic permission applies to the `_who`, `_where`, `_permissionId`, so we return `false`.
         return false;
     }
 

From fd20ad5d7cf44015459accd9212fcdaf1b257875 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Thu, 21 Sep 2023 12:25:13 +0200
Subject: [PATCH 35/38] fix: adapt test to the new behaviour

---
 packages/contracts/test/core/dao/dao.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index 63b128cc6..2b6f7ddef 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -1388,11 +1388,11 @@ describe('DAO', function () {
           ).to.equal(VALID_ERC1271_SIGNATURE);
         });
 
-        it('returns valid if only the generic condition is met', async () => {
+        it('returns invalid if the specific condition is not met although the generic condition is met', async () => {
           await specificMockCondition.setAnswer(false);
           expect(
             await dao.connect(caller).isValidSignature(hash, signature)
-          ).to.equal(VALID_ERC1271_SIGNATURE);
+          ).to.equal(INVALID_ERC1271_SIGNATURE);
         });
 
         it('returns invalid if both conditions are not met', async () => {

From 138cd2afcd0ca44e35c6d58c1f0bc041bc45fa08 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Thu, 21 Sep 2023 14:00:23 +0200
Subject: [PATCH 36/38] docs: improved comment

---
 packages/contracts/test/core/dao/dao.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts
index 2b6f7ddef..95c36f226 100644
--- a/packages/contracts/test/core/dao/dao.ts
+++ b/packages/contracts/test/core/dao/dao.ts
@@ -1388,7 +1388,7 @@ describe('DAO', function () {
           ).to.equal(VALID_ERC1271_SIGNATURE);
         });
 
-        it('returns invalid if the specific condition is not met although the generic condition is met', async () => {
+        it('returns invalid if the specific condition is not met although the generic condition is met (no fallback)', async () => {
           await specificMockCondition.setAnswer(false);
           expect(
             await dao.connect(caller).isValidSignature(hash, signature)

From dcca5f3bf95b548bd9319c0a0dd913fc352487b7 Mon Sep 17 00:00:00 2001
From: Michael Heuer <20623991+Michael-A-Heuer@users.noreply.github.com>
Date: Thu, 21 Sep 2023 15:13:57 +0200
Subject: [PATCH 37/38] docs: apply suggestions from code review

Co-authored-by: Mathias Scherer <mathias@aragon.org>
---
 packages/contracts/src/core/permission/PermissionManager.sol | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/contracts/src/core/permission/PermissionManager.sol b/packages/contracts/src/core/permission/PermissionManager.sol
index 97a10c5d1..f39721e64 100644
--- a/packages/contracts/src/core/permission/PermissionManager.sol
+++ b/packages/contracts/src/core/permission/PermissionManager.sol
@@ -209,11 +209,11 @@ abstract contract PermissionManager is Initializable {
         }
     }
 
-    /// @notice Checks if caller address has permission on target contract via a permission identifier and relays the answer to a condition contract if this was declared during the the granting process.
+    /// @notice Checks if the caller address has permission on the target contract via a permission identifier and relays the answer to a condition contract if this was declared during the granting process.
     /// @param _where The address of the target contract for which `_who` receives permission.
     /// @param _who The address (EOA or contract) for which the permission is checked.
     /// @param _permissionId The permission identifier.
-    /// @param _data Optional data to be passed to a referenced `PermissionCondition`.
+    /// @param _data Optional data to be passed to the set `PermissionCondition`.
     /// @return Returns true if `_who` has the permissions on the target contract via the specified permission identifier.
     function isGranted(
         address _where,

From f9aab5d09b636af7735c106c92f60408ee900d94 Mon Sep 17 00:00:00 2001
From: Michael Heuer <michael.heuer@rwth-aachen.de>
Date: Thu, 21 Sep 2023 15:17:56 +0200
Subject: [PATCH 38/38] typo: removed redundant comment signs

---
 packages/contracts/test/core/permission/permission-manager.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/packages/contracts/test/core/permission/permission-manager.ts b/packages/contracts/test/core/permission/permission-manager.ts
index 321a98523..7a5df98c3 100644
--- a/packages/contracts/test/core/permission/permission-manager.ts
+++ b/packages/contracts/test/core/permission/permission-manager.ts
@@ -962,7 +962,6 @@ describe('Core: PermissionManager', function () {
           genericTargetCondition.address
         )
       ).to.be.false;
-      //
       expect(
         await pm.isGranted(
           pm.address,