fix: verify state transition for pause guardian (TRST-R03)

Signed-off-by: Tomás Migone <[email protected]>

Author: tmigone

packages/horizon/contracts/data-service/extensions/DataServicePausable.sol

@@ -51,6 +51,10 @@ abstract contract DataServicePausable is Pausable, DataService, IDataServicePaus
      * @param _allowed The allowed status of the pause guardian
      */
     function _setPauseGuardian(address _pauseGuardian, bool _allowed) internal {
+        require(
+            pauseGuardians[_pauseGuardian] == !_allowed,
+            DataServicePausablePauseGuardianNoChange(_pauseGuardian, _allowed)
+        );
         pauseGuardians[_pauseGuardian] = _allowed;
         emit PauseGuardianSet(_pauseGuardian, _allowed);
     }

packages/horizon/contracts/data-service/extensions/DataServicePausableUpgradeable.sol

@@ -62,6 +62,10 @@ abstract contract DataServicePausableUpgradeable is PausableUpgradeable, DataSer
      * @param _allowed The allowed status of the pause guardian
      */
     function _setPauseGuardian(address _pauseGuardian, bool _allowed) internal whenNotPaused {
+        require(
+            pauseGuardians[_pauseGuardian] == !_allowed,
+            DataServicePausablePauseGuardianNoChange(_pauseGuardian, _allowed)
+        );
         pauseGuardians[_pauseGuardian] = _allowed;
         emit PauseGuardianSet(_pauseGuardian, _allowed);
     }

packages/horizon/contracts/data-service/interfaces/IDataServicePausable.sol

@@ -23,6 +23,13 @@ interface IDataServicePausable is IDataService {
      */
     error DataServicePausableNotPauseGuardian(address account);
 
+    /**
+     * @notice Emitted when a pause guardian is set to the same allowed status
+     * @param account The address of the pause guardian
+     * @param allowed The allowed status of the pause guardian
+     */
+    error DataServicePausablePauseGuardianNoChange(address account, bool allowed);
+
     /**
      * @notice Pauses the data service.
      * @dev Note that only functions using the modifiers `whenNotPaused`

packages/horizon/test/data-service/extensions/DataServicePausable.t.sol

@@ -70,6 +70,23 @@ contract DataServicePausableTest is HorizonStakingSharedTest {
         _assert_setPauseGuardian(address(this), false);
     }
 
+    function test_SetPauseGuardian_RevertWhen_AlreadyPauseGuardian() external {
+        _assert_setPauseGuardian(address(this), true);
+        vm.expectRevert(
+            abi.encodeWithSignature("DataServicePausablePauseGuardianNoChange(address,bool)", address(this), true)
+        );
+        dataService.setPauseGuardian(address(this), true);
+    }
+
+    function test_SetPauseGuardian_RevertWhen_AlreadyNotPauseGuardian() external {
+        _assert_setPauseGuardian(address(this), true);
+        _assert_setPauseGuardian(address(this), false);
+        vm.expectRevert(
+            abi.encodeWithSignature("DataServicePausablePauseGuardianNoChange(address,bool)", address(this), false)
+        );
+        dataService.setPauseGuardian(address(this), false);
+    }
+
     function test_PausedProtectedFn_RevertWhen_TheProtocolIsPaused() external {
         _assert_setPauseGuardian(address(this), true);
         _assert_pause();