pause testing

contracts/lib/Errors.sol

@@ -354,7 +354,7 @@ library Errors {
     //                            ProtocolPauser                              //
     ////////////////////////////////////////////////////////////////////////////
     error ProtocolPauser__ZeroAddress();
-    error ProtocolPauser__NotPausable();
+    error ProtocolPauser__AddingPausedContract();
     error ProtocolPauser__PausableAlreadyAdded();
     error ProtocolPauser__PausableNotFound();
 }

contracts/modules/royalty/policies/IpRoyaltyVault.sol

@@ -172,7 +172,7 @@ contract IpRoyaltyVault is IIpRoyaltyVault, ERC20SnapshotUpgradeable, Reentrancy
 
     /// @notice Allows token holders to claim revenue token based on the token balance at certain snapshot
     /// @param snapshotId The snapshot id
-    /// @param tokens The list of revenue tokens to claim
+    /// @param tokenList The list of revenue tokens to claim
     function claimRevenueByTokenBatch(uint256 snapshotId, address[] calldata tokenList) external nonReentrant whenNotPaused {
         IpRoyaltyVaultStorage storage $ = _getIpRoyaltyVaultStorage();
 

contracts/pause/ProtocolPausableUpgradeable.sol

@@ -5,11 +5,9 @@ pragma solidity 0.8.23;
 import { AccessManagedUpgradeable } from "@openzeppelin/contracts-upgradeable/access/manager/AccessManagedUpgradeable.sol";
 import { PausableUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol";
 
-import { IPausable } from "../interfaces/pause/IPausable.sol";
-
 /// @title ProtocolPausable
 /// @notice Contract that allows the pausing and unpausing of the a contract
-abstract contract ProtocolPausableUpgradeable is IPausable, PausableUpgradeable, AccessManagedUpgradeable {
+abstract contract ProtocolPausableUpgradeable is PausableUpgradeable, AccessManagedUpgradeable {
     /// @notice Initializes the ProtocolPausable contract
     /// @param accessManager The address of the access manager
     function __ProtocolPausable_init(address accessManager) public initializer {
@@ -18,16 +16,16 @@ abstract contract ProtocolPausableUpgradeable is IPausable, PausableUpgradeable,
     }
 
     /// @notice sets paused state
-    function pause() external override restricted {
+    function pause() external restricted {
         _pause();
     }
 
     /// @notice unsets unpaused state
-    function unpause() external override restricted {
+    function unpause() external restricted {
         _unpause();
     }
 
-    function paused() public view override(IPausable, PausableUpgradeable) returns (bool) {
+    function paused() public view override returns (bool) {
         return super.paused();
     }
 }

contracts/pause/ProtocolPauser.sol

@@ -2,11 +2,10 @@
 pragma solidity 0.8.23;
 
 import { AccessManaged } from "@openzeppelin/contracts/access/manager/AccessManaged.sol";
-import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
 import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
+import { ProtocolPausableUpgradeable } from "./ProtocolPausableUpgradeable.sol";
 
 import { IProtocolPauser } from "../interfaces/pause/IProtocolPauser.sol";
-import { IPausable } from "../interfaces/pause/IPausable.sol";
 import { Errors } from "../lib/Errors.sol";
 
 /// @title ProtocolPauser
@@ -16,7 +15,6 @@ import { Errors } from "../lib/Errors.sol";
 /// NOTE: If a contract is upgraded to remove the `IPausable` interface, it should be removed from the list of pausables
 /// before the upgrade, otherwise pause() and unpause() will revert.
 contract ProtocolPauser is IProtocolPauser, AccessManaged {
-    using ERC165Checker for address;
     using EnumerableSet for EnumerableSet.AddressSet;
 
     EnumerableSet.AddressSet private _pausables;
@@ -29,8 +27,8 @@ contract ProtocolPauser is IProtocolPauser, AccessManaged {
         if (pausable == address(0)) {
             revert Errors.ProtocolPauser__ZeroAddress();
         }
-        if (!pausable.supportsInterface(type(IPausable).interfaceId)) {
-            revert Errors.ProtocolPauser__NotPausable();
+        if (ProtocolPausableUpgradeable(pausable).paused()) {
+            revert Errors.ProtocolPauser__AddingPausedContract();
         }
         if (!_pausables.add(pausable)) {
             revert Errors.ProtocolPauser__PausableAlreadyAdded();
@@ -53,7 +51,7 @@ contract ProtocolPauser is IProtocolPauser, AccessManaged {
     function pause() external restricted {
         uint256 length = _pausables.length();
         for (uint256 i = 0; i < length; i++) {
-            IPausable(_pausables.at(i)).pause();
+            ProtocolPausableUpgradeable(_pausables.at(i)).pause();
         }
         emit ProtocolPaused();
     }
@@ -62,7 +60,7 @@ contract ProtocolPauser is IProtocolPauser, AccessManaged {
     function unpause() external restricted {
         uint256 length = _pausables.length();
         for (uint256 i = 0; i < length; i++) {
-            IPausable(_pausables.at(i)).unpause();
+            ProtocolPausableUpgradeable(_pausables.at(i)).unpause();
         }
         emit ProtocolUnpaused();
     }
@@ -71,7 +69,7 @@ contract ProtocolPauser is IProtocolPauser, AccessManaged {
     function isAllProtocolPaused() external view returns (bool) {
         uint256 length = _pausables.length();
         for (uint256 i = 0; i < length; i++) {
-            if (!IPausable(_pausables.at(i)).paused()) {
+            if (!ProtocolPausableUpgradeable(_pausables.at(i)).paused()) {
                 return false;
             }
         }

script/foundry/utils/DeployHelper.sol

@@ -470,6 +470,7 @@ contract DeployHelper is Script, BroadcastManager, JsonDeploymentHandler, Storag
         ///////// Role Granting /////////
         protocolAccessManager.grantRole(ProtocolAdmin.UPGRADER_ROLE, multisig, upgraderExecDelay);
         protocolAccessManager.grantRole(ProtocolAdmin.PAUSE_ADMIN_ROLE, multisig, 0);
+        protocolAccessManager.grantRole(ProtocolAdmin.PAUSE_ADMIN_ROLE, address(protocolPauser), 0);
         protocolAccessManager.grantRole(ProtocolAdmin.PROTOCOL_ADMIN_ROLE, multisig, 0);
 
         ///////// Renounce admin role /////////

test/foundry/mocks/MockProtocolPausable.sol

@@ -4,7 +4,7 @@ pragma solidity 0.8.23;
 import { ProtocolPausableUpgradeable } from "contracts/pause/ProtocolPausableUpgradeable.sol";
 
 contract MockProtocolPausable is ProtocolPausableUpgradeable {
-    function __MockPausable_init(address accessManager) public initializer {
+    function initialize(address accessManager) public initializer {
         __ProtocolPausable_init(accessManager);
     }
 }

test/foundry/pause/ProtocolPauser.t.sol

@@ -5,12 +5,26 @@ import { Errors } from "contracts/lib/Errors.sol";
 import { ProtocolAdmin } from "contracts/lib/ProtocolAdmin.sol";
 import { IProtocolPauser } from "contracts/interfaces/pause/IProtocolPauser.sol";
 
+import { IAccessManaged } from "@openzeppelin/contracts/access/manager/IAccessManaged.sol";
+
 import { BaseTest } from "../utils/BaseTest.t.sol";
 import { MockProtocolPausable } from "../mocks/MockProtocolPausable.sol";
+import { TestProxyHelper } from "../utils/TestProxyHelper.sol";
 
 contract ProtocolPauserTest is BaseTest {
 
-    MockProtocolPausable pausable = new MockProtocolPausable();
+    MockProtocolPausable pausable;
+
+    function setUp() public override {
+        super.setUp();
+        address impl = address(new MockProtocolPausable());
+        pausable = MockProtocolPausable(
+            TestProxyHelper.deployUUPSProxy(
+                impl,
+                abi.encodeCall(MockProtocolPausable.initialize, address(protocolAccessManager))
+            )
+        );
+    }
 
     function test_protocolPauser_validate_config() public {
         assertTrue(protocolPauser.isPausableRegistered(address(accessController)));
@@ -31,60 +45,70 @@ contract ProtocolPauserTest is BaseTest {
 
     function test_protocolPauser_addPausable_revert_zero() public {
         vm.prank(u.admin);
-        vm.expectRevert(Errors.ProtocolPauser__ZeroAddress());
+        vm.expectRevert(Errors.ProtocolPauser__ZeroAddress.selector);
         protocolPauser.addPausable(address(0));
     }
 
     function test_protocolPauser_addPausable_revert_notPausable() public {
         vm.prank(u.admin);
-        vm.expectRevert(Errors.ProtocolPauser__NotPausable());
+        vm.expectRevert();
         protocolPauser.addPausable(address(u.admin));
     }
 
+    function test_protocolPauser_addPausable_revert_paused() public {
+        vm.startPrank(u.admin);
+        pausable.pause();
+        vm.expectRevert();
+        protocolPauser.addPausable(address(pausable));
+        vm.stopPrank();
+    }
+
     function test_protocolPauser_addPausable_revert_alreadyAdded() public {
         vm.prank(u.admin);
-        vm.expectRevert(Errors.ProtocolPauser__PausableAlreadyAdded());
+        vm.expectRevert(Errors.ProtocolPauser__PausableAlreadyAdded.selector);
         protocolPauser.addPausable(address(licensingModule));
     }
 
     function test_protocolPauser_addPausable_revert_notAdmin() public {
-        vm.expectRevert("");
+        vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, address(this)));
         protocolPauser.addPausable(address(protocolPauser));
     }
 
     function test_protocolPauser_removePausable() public {
+        vm.startPrank(u.admin);
+        protocolPauser.addPausable(address(pausable));
+
         vm.expectEmit();
-        emit IProtocolPauser.PausableRemoved(address(protocolPauser));
-        vm.prank(u.admin);
-        protocolPauser.removePausable(address(protocolPauser));
-        assertFalse(protocolPauser.isPausableRegistered(address(protocolPauser)));
+        emit IProtocolPauser.PausableRemoved(address(pausable));
+        protocolPauser.removePausable(address(pausable));
+        assertFalse(protocolPauser.isPausableRegistered(address(pausable)));
+        vm.stopPrank();
     }
 
     function test_protocolPauser_removePausable_notFound() public {
-        vm.expectRevert("");
-        vm.prank(u.bob);
+        vm.prank(u.admin);
+        vm.expectRevert(Errors.ProtocolPauser__PausableNotFound.selector);
         protocolPauser.removePausable(address(u.admin));
     }
 
     function test_ProtocolPauser_pause() public {
         vm.prank(u.admin);
-        protocolAccessManager.grantRole(ProtocolAdmin.PAUSE_ADMIN_ROLE, address(u.bob));
+        protocolAccessManager.grantRole(ProtocolAdmin.PAUSE_ADMIN_ROLE, address(u.bob), 0);
 
         vm.prank(u.bob);
         vm.expectEmit();
-        emit IProtocolPauser.ProtocolPaused();
         protocolPauser.pause();
         assertTrue(protocolPauser.isAllProtocolPaused());
     }
 
     function test_ProtocolPauser_pause_revert_notPauser() public {
-        vm.expectRevert("");
+        vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, address(this)));
         protocolPauser.pause();
     }
 
     function test_ProtocolPauser_unpause() public {
         vm.prank(u.admin);
-        protocolAccessManager.grantRole(ProtocolAdmin.PAUSE_ADMIN_ROLE, address(u.bob));
+        protocolAccessManager.grantRole(ProtocolAdmin.PAUSE_ADMIN_ROLE, u.bob, 0);
 
         vm.startPrank(u.bob);
         protocolPauser.pause();