Adapting the tests

packages/contracts/src/conditions/MemberAccessExecuteCondition.sol

@@ -28,7 +28,7 @@ contract MemberAccessExecuteCondition is PermissionCondition {
         (_where, _who, _permissionId);
 
         // Is it execute()?
-        if (getSelector(_data) != IDAO.execute.selector) {
+        if (_getSelector(_data) != IDAO.execute.selector) {
             return false;
         }
 
@@ -42,7 +42,7 @@ contract MemberAccessExecuteCondition is PermissionCondition {
         else if (_actions[0].to != targetContract) return false;
 
         // Decode the call being requested (both have the same parameters)
-        (bytes4 _requestedSelector, ) = decodeAddRemoveMemberCalldata(_actions[0].data);
+        (bytes4 _requestedSelector, ) = _decodeAddRemoveMemberCalldata(_actions[0].data);
 
         if (
             _requestedSelector != MainVotingPlugin.addMember.selector &&
@@ -52,15 +52,15 @@ contract MemberAccessExecuteCondition is PermissionCondition {
         return true;
     }
 
-    function getSelector(bytes memory _data) internal pure returns (bytes4 selector) {
+    function _getSelector(bytes memory _data) internal pure returns (bytes4 selector) {
         // Slices are only supported for bytes calldata, not bytes memory
         // Bytes memory requires an assembly block
         assembly {
             selector := mload(add(_data, 0x20)) // 32
         }
     }
 
-    function decodeAddRemoveMemberCalldata(
+    function _decodeAddRemoveMemberCalldata(
         bytes memory _data
     ) internal pure returns (bytes4 sig, address account) {
         // Slicing is only supported for bytes calldata, not bytes memory

packages/contracts/src/conditions/OnlyPluginUpgraderCondition.sol

@@ -53,7 +53,7 @@ contract OnlyPluginUpgraderCondition is PermissionCondition {
     ) external view returns (bool) {
         (_where, _who, _permissionId);
 
-        bytes4 _requestedFuncSig = getSelector(_data);
+        bytes4 _requestedFuncSig = _getSelector(_data);
         if (_requestedFuncSig != IDAO.execute.selector) {
             return false;
         }
@@ -69,26 +69,28 @@ contract OnlyPluginUpgraderCondition is PermissionCondition {
 
         // Action 1/3: GRANT/REVOKE UPGRADE_PLUGIN_PERMISSION_ID to the PSP on targetPlugis[]
         if (_actions[0].to != dao || _actions[2].to != dao) return false;
-        else if (!isValidGrantRevokeCalldata(_actions[0].data, _actions[2].data)) return false;
+        else if (!_isValidGrantRevokeCalldata(_actions[0].data, _actions[2].data)) return false;
 
         // Action 2: CALL PSP.applyUpdate() onto targetPlugins[]
         if (_actions[1].to != psp) return false;
-        else if (!isValidApplyUpdateCalldata(_actions[1].data)) return false;
+        else if (!_isValidApplyUpdateCalldata(_actions[1].data)) return false;
 
         return true;
     }
 
-    function getSelector(bytes memory _data) public pure returns (bytes4 selector) {
+    // Internal helpers
+
+    function _getSelector(bytes memory _data) internal pure returns (bytes4 selector) {
         // Slices are only supported for bytes calldata
         // Bytes memory requires an assembly block
         assembly {
             selector := mload(add(_data, 0x20)) // 32
         }
     }
 
-    function decodeGrantRevokeCalldata(
+    function _decodeGrantRevokeCalldata(
         bytes memory _data
-    ) public pure returns (bytes4 selector, address where, address who, bytes32 permissionId) {
+    ) internal pure returns (bytes4 selector, address where, address who, bytes32 permissionId) {
         // Slicing is only supported for bytes calldata, not bytes memory
         // Bytes memory requires an assembly block
         assembly {
@@ -99,9 +101,9 @@ contract OnlyPluginUpgraderCondition is PermissionCondition {
         }
     }
 
-    function decodeApplyUpdateCalldata(
+    function _decodeApplyUpdateCalldata(
         bytes memory _data
-    ) public pure returns (bytes4 selector, address daoAddress, address targetPluginAddress) {
+    ) internal pure returns (bytes4 selector, address daoAddress, address targetPluginAddress) {
         // Slicing is only supported for bytes calldata, not bytes memory
         // Bytes memory requires an assembly block
         assembly {
@@ -124,19 +126,17 @@ contract OnlyPluginUpgraderCondition is PermissionCondition {
         }
     }
 
-    // Internal helpers
-
-    function isValidGrantRevokeCalldata(
+    function _isValidGrantRevokeCalldata(
         bytes memory _grantData,
         bytes memory _revokeData
-    ) private view returns (bool) {
+    ) internal view returns (bool) {
         // Grant checks
         (
             bytes4 _grantSelector,
             address _grantWhere,
             address _grantWho,
             bytes32 _grantPermission
-        ) = decodeGrantRevokeCalldata(_grantData);
+        ) = _decodeGrantRevokeCalldata(_grantData);
 
         if (_grantSelector != PermissionManager.grant.selector) return false;
         else if (!allowedPluginAddresses[_grantWhere]) return false;
@@ -149,7 +149,7 @@ contract OnlyPluginUpgraderCondition is PermissionCondition {
             address _revokeWhere,
             address _revokeWho,
             bytes32 _revokePermission
-        ) = decodeGrantRevokeCalldata(_revokeData);
+        ) = _decodeGrantRevokeCalldata(_revokeData);
 
         if (_revokeSelector != PermissionManager.revoke.selector) return false;
         else if (!allowedPluginAddresses[_revokeWhere]) return false;
@@ -162,8 +162,8 @@ contract OnlyPluginUpgraderCondition is PermissionCondition {
         return true;
     }
 
-    function isValidApplyUpdateCalldata(bytes memory _data) private view returns (bool) {
-        (bytes4 _selector, address _dao, address _targetPluginAddress) = decodeApplyUpdateCalldata(
+    function _isValidApplyUpdateCalldata(bytes memory _data) internal view returns (bool) {
+        (bytes4 _selector, address _dao, address _targetPluginAddress) = _decodeApplyUpdateCalldata(
             _data
         );
 

packages/contracts/src/test/TestMemberAccessExecuteCondition.sol

@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+pragma solidity 0.8.17;
+
+import {IDAO} from "@aragon/osx/core/dao/IDAO.sol";
+import {MemberAccessExecuteCondition} from "../conditions/MemberAccessExecuteCondition.sol";
+
+/// @notice The condition associated with `TestSharedPlugin`
+contract TestMemberAccessExecuteCondition is MemberAccessExecuteCondition {
+    constructor(address _targetContract) MemberAccessExecuteCondition(_targetContract) {}
+
+    function getSelector(bytes memory _data) public pure returns (bytes4 selector) {
+        return super._getSelector(_data);
+    }
+
+    function decodeAddRemoveMemberCalldata(
+        bytes memory _data
+    ) public pure returns (bytes4 sig, address account) {
+        return super._decodeAddRemoveMemberCalldata(_data);
+    }
+}

packages/contracts/src/test/TestOnlyPluginUpgraderCondition.sol

@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+pragma solidity 0.8.17;
+
+import {DAO} from "@aragon/osx/core/dao/DAO.sol";
+import {PluginSetupProcessor} from "@aragon/osx/framework/plugin/setup/PluginSetupProcessor.sol";
+import {OnlyPluginUpgraderCondition} from "../conditions/OnlyPluginUpgraderCondition.sol";
+
+/// @notice The condition associated with `TestSharedPlugin`
+contract TestOnlyPluginUpgraderCondition is OnlyPluginUpgraderCondition {
+    constructor(
+        DAO _dao,
+        PluginSetupProcessor _psp,
+        address[] memory _targetPluginAddresses
+    ) OnlyPluginUpgraderCondition(_dao, _psp, _targetPluginAddresses) {}
+
+    function getSelector(bytes memory _data) public pure returns (bytes4 selector) {
+        return super._getSelector(_data);
+    }
+
+    function decodeGrantRevokeCalldata(
+        bytes memory _data
+    ) public pure returns (bytes4 selector, address where, address who, bytes32 permissionId) {
+        return super._decodeGrantRevokeCalldata(_data);
+    }
+
+    function decodeApplyUpdateCalldata(
+        bytes memory _data
+    ) public pure returns (bytes4 selector, address daoAddress, address targetPluginAddress) {
+        return super._decodeApplyUpdateCalldata(_data);
+    }
+
+    function isValidGrantRevokeCalldata(
+        bytes memory _grantData,
+        bytes memory _revokeData
+    ) public view returns (bool) {
+        return super._isValidGrantRevokeCalldata(_grantData, _revokeData);
+    }
+
+    function isValidApplyUpdateCalldata(bytes memory _data) public view returns (bool) {
+        return super._isValidApplyUpdateCalldata(_data);
+    }
+}

packages/contracts/test/unit-testing/member-access-execute-condition.ts

@@ -5,6 +5,8 @@ import {
   MainVotingPlugin__factory,
   MemberAccessExecuteCondition,
   MemberAccessExecuteCondition__factory,
+  TestMemberAccessExecuteCondition__factory,
+  TestMemberAccessExecuteCondition,
 } from '../../typechain';
 import {getPluginSetupProcessorAddress} from '../../utils/helpers';
 import {deployTestDao} from '../helpers/test-dao';
@@ -313,7 +315,16 @@ describe('Member Access Condition', function () {
     });
   });
 
-  describe('Decoders', () => {
+  describe('Decoders (internal)', () => {
+    let testMemberAccessExecuteCondition: TestMemberAccessExecuteCondition;
+
+    beforeEach(async () => {
+      const factory = new TestMemberAccessExecuteCondition__factory(alice);
+      testMemberAccessExecuteCondition = await factory.deploy(
+        SOME_CONTRACT_ADDRESS
+      );
+    });
+
     it('Should decode getSelector properly', async () => {
       const actions: IDAO.ActionStruct[] = [
         {
@@ -333,15 +344,20 @@ describe('Member Access Condition', function () {
       ];
 
       expect(
-        await memberAccessExecuteCondition.getSelector(actions[0].data)
+        await testMemberAccessExecuteCondition.getSelector(actions[0].data)
       ).to.eq((actions[0].data as string).slice(0, 10));
 
       expect(
-        await memberAccessExecuteCondition.getSelector(actions[1].data)
+        await testMemberAccessExecuteCondition.getSelector(actions[1].data)
       ).to.eq((actions[1].data as string).slice(0, 10));
     });
 
     it('Should decode decodeGrantRevokeCalldata properly', async () => {
+      const factory = new TestMemberAccessExecuteCondition__factory(alice);
+      const testMemberAccessExecuteCondition = await factory.deploy(
+        SOME_CONTRACT_ADDRESS
+      );
+
       const calldataList = [
         mainVotingPluginInterface.encodeFunctionData('addMember', [pspAddress]),
         mainVotingPluginInterface.encodeFunctionData('removeMember', [
@@ -351,15 +367,15 @@ describe('Member Access Condition', function () {
 
       // 1
       let [selector, who] =
-        await memberAccessExecuteCondition.decodeAddRemoveMemberCalldata(
+        await testMemberAccessExecuteCondition.decodeAddRemoveMemberCalldata(
           calldataList[0]
         );
       expect(selector).to.eq(calldataList[0].slice(0, 10));
       expect(who).to.eq(pspAddress);
 
       // 2
       [selector, who] =
-        await memberAccessExecuteCondition.decodeAddRemoveMemberCalldata(
+        await testMemberAccessExecuteCondition.decodeAddRemoveMemberCalldata(
           calldataList[1]
         );
       expect(selector).to.eq(calldataList[1].slice(0, 10));

packages/contracts/test/unit-testing/only-plugin-upgrader-condition.ts

@@ -6,6 +6,8 @@ import {
   IDAO,
   OnlyPluginUpgraderCondition,
   OnlyPluginUpgraderCondition__factory,
+  TestOnlyPluginUpgraderCondition__factory,
+  TestOnlyPluginUpgraderCondition,
 } from '../../typechain';
 import {getPluginSetupProcessorAddress} from '../../utils/helpers';
 import {deployTestDao} from '../helpers/test-dao';
@@ -1192,7 +1194,18 @@ describe('Only Plugin Upgrader Condition', function () {
     });
   });
 
-  describe('Decoders', () => {
+  describe('Decoders (internal)', () => {
+    let testMemberAccessExecuteCondition: TestOnlyPluginUpgraderCondition;
+
+    beforeEach(async () => {
+      const factory = new TestOnlyPluginUpgraderCondition__factory(alice);
+      testMemberAccessExecuteCondition = await factory.deploy(
+        dao.address,
+        pspAddress,
+        [ALLOWED_PLUGIN_ADDRESS_1, ALLOWED_PLUGIN_ADDRESS_2]
+      );
+    });
+
     it('Should decode getSelector properly', async () => {
       const actions: IDAO.ActionStruct[] = [
         {
@@ -1224,15 +1237,15 @@ describe('Only Plugin Upgrader Condition', function () {
       ];
 
       expect(
-        await onlyPluginUpgraderCondition.getSelector(actions[0].data)
+        await testMemberAccessExecuteCondition.getSelector(actions[0].data)
       ).to.eq((actions[0].data as string).slice(0, 10));
 
       expect(
-        await onlyPluginUpgraderCondition.getSelector(actions[1].data)
+        await testMemberAccessExecuteCondition.getSelector(actions[1].data)
       ).to.eq((actions[1].data as string).slice(0, 10));
 
       expect(
-        await onlyPluginUpgraderCondition.getSelector(actions[2].data)
+        await testMemberAccessExecuteCondition.getSelector(actions[2].data)
       ).to.eq((actions[2].data as string).slice(0, 10));
     });
 
@@ -1252,7 +1265,7 @@ describe('Only Plugin Upgrader Condition', function () {
 
       // 1
       let [selector, where, who, permissionId] =
-        await onlyPluginUpgraderCondition.decodeGrantRevokeCalldata(
+        await testMemberAccessExecuteCondition.decodeGrantRevokeCalldata(
           calldataList[0]
         );
       expect(selector).to.eq(calldataList[0].slice(0, 10));
@@ -1262,7 +1275,7 @@ describe('Only Plugin Upgrader Condition', function () {
 
       // 2
       [selector, where, who, permissionId] =
-        await onlyPluginUpgraderCondition.decodeGrantRevokeCalldata(
+        await testMemberAccessExecuteCondition.decodeGrantRevokeCalldata(
           calldataList[1]
         );
       expect(selector).to.eq(calldataList[1].slice(0, 10));
@@ -1289,7 +1302,7 @@ describe('Only Plugin Upgrader Condition', function () {
 
       // 1
       let [selector, decodedDaoAddress, pluginAddress] =
-        await onlyPluginUpgraderCondition.decodeApplyUpdateCalldata(
+        await testMemberAccessExecuteCondition.decodeApplyUpdateCalldata(
           calldataList[0]
         );
       expect(selector).to.eq(calldataList[0].slice(0, 10));
@@ -1298,7 +1311,7 @@ describe('Only Plugin Upgrader Condition', function () {
 
       // 2
       [selector, decodedDaoAddress, pluginAddress] =
-        await onlyPluginUpgraderCondition.decodeApplyUpdateCalldata(
+        await testMemberAccessExecuteCondition.decodeApplyUpdateCalldata(
           calldataList[1]
         );
       expect(selector).to.eq(calldataList[1].slice(0, 10));