[TIP-120] DAO Vote: PCVGuardian v3

contracts/libs/CoreRefPauseableLib.sol

@@ -34,6 +34,10 @@ library CoreRefPauseableLib {
         }
     }
 
+    function _paused(address _pauseableCoreRefAddress) internal view returns (bool) {
+        return CoreRef(_pauseableCoreRefAddress).paused();
+    }
+
     function _pause(address _pauseableCoreRefAddress) internal {
         CoreRef(_pauseableCoreRefAddress).pause();
     }

contracts/mock/MockPCVDepositV2.sol

@@ -33,12 +33,22 @@ contract MockPCVDepositV2 is IPCVDeposit, CoreRef {
         return (resistantBalance, resistantProtocolOwnedFei);
     }
 
+    // gets the resistant token balance of this deposit
+    function _resistantBalance() external view returns (uint256) {
+        return resistantBalance;
+    }
+
+    // gets the resistant protocol owned fei of this deposit
+    function _resistantFei() external view returns (uint256) {
+        return resistantProtocolOwnedFei;
+    }
+
     // IPCVDeposit V1
-    function deposit() external override {
+    function deposit() external override whenNotPaused {
         resistantBalance = IERC20(balanceReportedIn).balanceOf(address(this));
     }
 
-    function withdraw(address to, uint256 amount) external override {
+    function withdraw(address to, uint256 amount) external override whenNotPaused {
         IERC20(balanceReportedIn).transfer(to, amount);
         resistantBalance = IERC20(balanceReportedIn).balanceOf(address(this));
     }

contracts/pcv/IPCVGuardian.sol

@@ -50,47 +50,82 @@ interface IPCVGuardian {
     /// @param safeAddresses the addresses to un-set as safe
     function unsetSafeAddresses(address[] calldata safeAddresses) external;
 
-    /// @notice governor-or-guardian-only method to withdraw funds from a pcv deposit, by calling the withdraw() method on it
+    /// @notice withdraw funds from a pcv deposit, by calling the withdraw() method on it
     /// @param pcvDeposit the address of the pcv deposit contract
     /// @param safeAddress the destination address to withdraw to
     /// @param amount the amount to withdraw
-    /// @param pauseAfter if true, the pcv contract will be paused after the withdraw
     /// @param depositAfter if true, attempts to deposit to the target PCV deposit
     function withdrawToSafeAddress(
         address pcvDeposit,
         address safeAddress,
         uint256 amount,
-        bool pauseAfter,
         bool depositAfter
     ) external;
 
-    /// @notice governor-or-guardian-only method to withdraw funds from a pcv deposit, by calling the withdraw() method on it
+    /// @notice withdraw funds from a pcv deposit, by calling the withdraw() method on it.
+    /// The amount withdrawn is expressed as a ratio, basis points are given & not absolute value.
+    /// @param pcvDeposit the address of the pcv deposit contract
+    /// @param safeAddress the destination address to withdraw to
+    /// @param basisPoints the percent in basis points [1-10000] if the deposit's balance to withdraw
+    /// @param depositAfter if true, attempts to deposit to the target PCV deposit
+    function withdrawRatioToSafeAddress(
+        address pcvDeposit,
+        address safeAddress,
+        uint256 basisPoints,
+        bool depositAfter
+    ) external;
+
+    /// @notice withdraw funds from a pcv deposit, by calling the withdrawETH() method on it
     /// @param pcvDeposit the address of the pcv deposit contract
     /// @param safeAddress the destination address to withdraw to
     /// @param amount the amount of tokens to withdraw
-    /// @param pauseAfter if true, the pcv contract will be paused after the withdraw
     /// @param depositAfter if true, attempts to deposit to the target PCV deposit
     function withdrawETHToSafeAddress(
         address pcvDeposit,
         address payable safeAddress,
         uint256 amount,
-        bool pauseAfter,
         bool depositAfter
     ) external;
 
-    /// @notice governor-or-guardian-only method to withdraw funds from a pcv deposit, by calling the withdraw() method on it
+    /// @notice withdraw funds from a pcv deposit, by calling the withdrawETH() method on it.
+    /// The amount withdrawn is expressed as a ratio, basis points are given & not absolute value.
+    /// @param pcvDeposit the address of the pcv deposit contract
+    /// @param safeAddress the destination address to withdraw to
+    /// @param basisPoints the percent in basis points [1-10000] if the deposit's balance to withdraw
+    /// @param depositAfter if true, attempts to deposit to the target PCV deposit
+    function withdrawETHRatioToSafeAddress(
+        address pcvDeposit,
+        address payable safeAddress,
+        uint256 basisPoints,
+        bool depositAfter
+    ) external;
+
+    /// @notice withdraw funds from a pcv deposit, by calling the withdrawERC20() method on it
     /// @param pcvDeposit the deposit to pull funds from
     /// @param safeAddress the destination address to withdraw to
     /// @param token the token to withdraw
     /// @param amount the amount of funds to withdraw
-    /// @param pauseAfter whether to pause the pcv after withdrawing
     /// @param depositAfter if true, attempts to deposit to the target PCV deposit
     function withdrawERC20ToSafeAddress(
         address pcvDeposit,
         address safeAddress,
         address token,
         uint256 amount,
-        bool pauseAfter,
+        bool depositAfter
+    ) external;
+
+    /// @notice withdraw funds from a pcv deposit, by calling the withdrawERC20() method on it.
+    /// The amount withdrawn is expressed as a ratio, basis points are given & not absolute value.
+    /// @param pcvDeposit the deposit to pull funds from
+    /// @param safeAddress the destination address to withdraw to
+    /// @param token the token to withdraw
+    /// @param basisPoints the percent in basis points [1-10000] if the deposit's balance to withdraw
+    /// @param depositAfter if true, attempts to deposit to the target PCV deposit
+    function withdrawERC20RatioToSafeAddress(
+        address pcvDeposit,
+        address safeAddress,
+        address token,
+        uint256 basisPoints,
         bool depositAfter
     ) external;
 }

contracts/pcv/PCVGuardian.sol

@@ -1,12 +1,15 @@
 // SPDX-License-Identifier: GPL-3.0-or-later
 pragma solidity ^0.8.4;
 
-import "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
-import "../refs/CoreRef.sol";
-import "./IPCVGuardian.sol";
-import "./IPCVDeposit.sol";
-import "../libs/CoreRefPauseableLib.sol";
+import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
+import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+import {ICore} from "../core/ICore.sol";
+import {CoreRef} from "../refs/CoreRef.sol";
+import {IPCVGuardian} from "./IPCVGuardian.sol";
+import {IPCVDeposit} from "./IPCVDeposit.sol";
+import {CoreRefPauseableLib} from "../libs/CoreRefPauseableLib.sol";
 import {TribeRoles} from "../core/TribeRoles.sol";
+import {Constants} from "../Constants.sol";
 
 contract PCVGuardian is IPCVGuardian, CoreRef {
     using CoreRefPauseableLib for address;
@@ -41,7 +44,7 @@ contract PCVGuardian is IPCVGuardian, CoreRef {
     function setSafeAddress(address pcvDeposit)
         external
         override
-        hasAnyOfTwoRoles(TribeRoles.GOVERNOR, TribeRoles.PCV_GUARDIAN_ADMIN)
+        hasAnyOfTwoRoles(TribeRoles.PCV_GUARDIAN_ADMIN, TribeRoles.GOVERNOR)
     {
         _setSafeAddress(pcvDeposit);
     }
@@ -51,9 +54,9 @@ contract PCVGuardian is IPCVGuardian, CoreRef {
     function setSafeAddresses(address[] calldata _safeAddresses)
         external
         override
-        hasAnyOfTwoRoles(TribeRoles.GOVERNOR, TribeRoles.PCV_GUARDIAN_ADMIN)
+        hasAnyOfTwoRoles(TribeRoles.PCV_GUARDIAN_ADMIN, TribeRoles.GOVERNOR)
     {
-        require(_safeAddresses.length != 0, "empty");
+        require(_safeAddresses.length != 0, "PCVGuardian: empty");
         for (uint256 i = 0; i < _safeAddresses.length; i++) {
             _setSafeAddress(_safeAddresses[i]);
         }
@@ -66,7 +69,7 @@ contract PCVGuardian is IPCVGuardian, CoreRef {
     function unsetSafeAddress(address pcvDeposit)
         external
         override
-        hasAnyOfThreeRoles(TribeRoles.GOVERNOR, TribeRoles.GUARDIAN, TribeRoles.PCV_GUARDIAN_ADMIN)
+        hasAnyOfThreeRoles(TribeRoles.PCV_GUARDIAN_ADMIN, TribeRoles.GUARDIAN, TribeRoles.GOVERNOR)
     {
         _unsetSafeAddress(pcvDeposit);
     }
@@ -76,114 +79,167 @@ contract PCVGuardian is IPCVGuardian, CoreRef {
     function unsetSafeAddresses(address[] calldata _safeAddresses)
         external
         override
-        hasAnyOfThreeRoles(TribeRoles.GOVERNOR, TribeRoles.GUARDIAN, TribeRoles.PCV_GUARDIAN_ADMIN)
+        hasAnyOfThreeRoles(TribeRoles.PCV_GUARDIAN_ADMIN, TribeRoles.GUARDIAN, TribeRoles.GOVERNOR)
     {
-        require(_safeAddresses.length != 0, "empty");
+        require(_safeAddresses.length != 0, "PCVGuardian: empty");
         for (uint256 i = 0; i < _safeAddresses.length; i++) {
             _unsetSafeAddress(_safeAddresses[i]);
         }
     }
 
-    /// @notice governor-or-guardian-only method to withdraw funds from a pcv deposit, by calling the withdraw() method on it
+    /// @notice withdraw funds from a pcv deposit, by calling the withdraw() method on it
     /// @param pcvDeposit the address of the pcv deposit contract
     /// @param safeAddress the destination address to withdraw to
     /// @param amount the amount to withdraw
-    /// @param pauseAfter if true, the pcv contract will be paused after the withdraw
     /// @param depositAfter if true, attempts to deposit to the target PCV deposit
     function withdrawToSafeAddress(
         address pcvDeposit,
         address safeAddress,
         uint256 amount,
-        bool pauseAfter,
         bool depositAfter
-    ) external override hasAnyOfThreeRoles(TribeRoles.GOVERNOR, TribeRoles.PCV_SAFE_MOVER_ROLE, TribeRoles.GUARDIAN) {
-        require(isSafeAddress(safeAddress), "Provided address is not a safe address!");
-
-        pcvDeposit._ensureUnpaused();
-
+    ) external override beforeAndAfterWithdrawal(pcvDeposit, safeAddress, depositAfter) {
         IPCVDeposit(pcvDeposit).withdraw(safeAddress, amount);
+        emit PCVGuardianWithdrawal(pcvDeposit, safeAddress, amount);
+    }
 
-        if (pauseAfter) {
-            pcvDeposit._pause();
-        }
-
-        if (depositAfter) {
-            IPCVDeposit(safeAddress).deposit();
-        }
+    /// @notice withdraw funds from a pcv deposit, by calling the withdraw() method on it
+    /// @param pcvDeposit the address of the pcv deposit contract
+    /// @param safeAddress the destination address to withdraw to
+    /// @param basisPoints the percent in basis points [1-10000] if the deposit's balance to withdraw
+    /// @param depositAfter if true, attempts to deposit to the target PCV deposit
+    function withdrawRatioToSafeAddress(
+        address pcvDeposit,
+        address safeAddress,
+        uint256 basisPoints,
+        bool depositAfter
+    ) external override beforeAndAfterWithdrawal(pcvDeposit, safeAddress, depositAfter) {
+        require(basisPoints <= Constants.BASIS_POINTS_GRANULARITY, "PCVGuardian: basisPoints too high");
+        uint256 amount = (IPCVDeposit(pcvDeposit).balance() * basisPoints) / Constants.BASIS_POINTS_GRANULARITY;
+        require(amount != 0, "PCVGuardian: no value to withdraw");
 
+        IPCVDeposit(pcvDeposit).withdraw(safeAddress, amount);
         emit PCVGuardianWithdrawal(pcvDeposit, safeAddress, amount);
     }
 
-    /// @notice governor-or-guardian-only method to withdraw funds from a pcv deposit, by calling the withdraw() method on it
+    /// @notice withdraw funds from a pcv deposit, by calling the withdrawETH() method on it
     /// @param pcvDeposit the address of the pcv deposit contract
     /// @param safeAddress the destination address to withdraw to
     /// @param amount the amount of tokens to withdraw
-    /// @param pauseAfter if true, the pcv contract will be paused after the withdraw
     /// @param depositAfter if true, attempts to deposit to the target PCV deposit
     function withdrawETHToSafeAddress(
         address pcvDeposit,
         address payable safeAddress,
         uint256 amount,
-        bool pauseAfter,
         bool depositAfter
-    ) external override hasAnyOfThreeRoles(TribeRoles.GOVERNOR, TribeRoles.PCV_SAFE_MOVER_ROLE, TribeRoles.GUARDIAN) {
-        require(isSafeAddress(safeAddress), "Provided address is not a safe address!");
-
-        pcvDeposit._ensureUnpaused();
-
+    ) external override beforeAndAfterWithdrawal(pcvDeposit, safeAddress, depositAfter) {
         IPCVDeposit(pcvDeposit).withdrawETH(safeAddress, amount);
+        emit PCVGuardianETHWithdrawal(pcvDeposit, safeAddress, amount);
+    }
 
-        if (pauseAfter) {
-            pcvDeposit._pause();
-        }
-
-        if (depositAfter) {
-            IPCVDeposit(safeAddress).deposit();
-        }
+    /// @notice withdraw funds from a pcv deposit, by calling the withdrawETH() method on it
+    /// @param pcvDeposit the address of the pcv deposit contract
+    /// @param safeAddress the destination address to withdraw to
+    /// @param basisPoints the percent in basis points [1-10000] if the deposit's balance to withdraw
+    /// @param depositAfter if true, attempts to deposit to the target PCV deposit
+    function withdrawETHRatioToSafeAddress(
+        address pcvDeposit,
+        address payable safeAddress,
+        uint256 basisPoints,
+        bool depositAfter
+    ) external override beforeAndAfterWithdrawal(pcvDeposit, safeAddress, depositAfter) {
+        require(basisPoints <= Constants.BASIS_POINTS_GRANULARITY, "PCVGuardian: basisPoints too high");
+        uint256 amount = (pcvDeposit.balance * basisPoints) / Constants.BASIS_POINTS_GRANULARITY;
+        require(amount != 0, "PCVGuardian: no value to withdraw");
 
+        IPCVDeposit(pcvDeposit).withdrawETH(safeAddress, amount);
         emit PCVGuardianETHWithdrawal(pcvDeposit, safeAddress, amount);
     }
 
-    /// @notice governor-or-guardian-only method to withdraw funds from a pcv deposit, by calling the withdraw() method on it
+    /// @notice withdraw funds from a pcv deposit, by calling the withdrawERC20() method on it
     /// @param pcvDeposit the deposit to pull funds from
     /// @param safeAddress the destination address to withdraw to
     /// @param amount the amount of funds to withdraw
-    /// @param pauseAfter whether to pause the pcv after withdrawing
     /// @param depositAfter if true, attempts to deposit to the target PCV deposit
     function withdrawERC20ToSafeAddress(
         address pcvDeposit,
         address safeAddress,
         address token,
         uint256 amount,
-        bool pauseAfter,
         bool depositAfter
-    ) external override hasAnyOfThreeRoles(TribeRoles.GOVERNOR, TribeRoles.PCV_SAFE_MOVER_ROLE, TribeRoles.GUARDIAN) {
-        require(isSafeAddress(safeAddress), "Provided address is not a safe address!");
+    ) external override beforeAndAfterWithdrawal(pcvDeposit, safeAddress, depositAfter) {
+        IPCVDeposit(pcvDeposit).withdrawERC20(token, safeAddress, amount);
+        emit PCVGuardianERC20Withdrawal(pcvDeposit, safeAddress, token, amount);
+    }
 
-        pcvDeposit._ensureUnpaused();
+    /// @notice withdraw funds from a pcv deposit, by calling the withdrawERC20() method on it
+    /// @param pcvDeposit the deposit to pull funds from
+    /// @param safeAddress the destination address to withdraw to
+    /// @param basisPoints the percent in basis points [1-10000] if the deposit's balance to withdraw
+    /// @param depositAfter if true, attempts to deposit to the target PCV deposit
+    function withdrawERC20RatioToSafeAddress(
+        address pcvDeposit,
+        address safeAddress,
+        address token,
+        uint256 basisPoints,
+        bool depositAfter
+    ) external override beforeAndAfterWithdrawal(pcvDeposit, safeAddress, depositAfter) {
+        require(basisPoints <= Constants.BASIS_POINTS_GRANULARITY, "PCVGuardian: basisPoints too high");
+        uint256 amount = (IERC20(token).balanceOf(pcvDeposit) * basisPoints) / Constants.BASIS_POINTS_GRANULARITY;
+        require(amount != 0, "PCVGuardian: no value to withdraw");
 
         IPCVDeposit(pcvDeposit).withdrawERC20(token, safeAddress, amount);
+        emit PCVGuardianERC20Withdrawal(pcvDeposit, safeAddress, token, amount);
+    }
+
+    // --------------- Modifiers  -------------
+
+    /// @notice modifier to factorize the logic in all withdrawals :
+    /// - first, ensure the deposit to withdraw from is unpaused
+    /// - second, perform withdrawal
+    /// - third, re-pause deposit if it was paused
+    /// - finally, call pcvDeposit.deposit() if depositAfter = true
+    modifier beforeAndAfterWithdrawal(
+        address pcvDeposit,
+        address safeAddress,
+        bool depositAfter
+    ) {
+        {
+            // scoped in this modifier to prevent stack to deep errors & enforce consistent acl
+            ICore _core = core();
+            require(
+                _core.hasRole(TribeRoles.GUARDIAN, msg.sender) ||
+                    _core.hasRole(TribeRoles.PCV_SAFE_MOVER_ROLE, msg.sender) ||
+                    _core.hasRole(TribeRoles.GOVERNOR, msg.sender),
+                "UNAUTHORIZED"
+            );
+            require(isSafeAddress(safeAddress), "PCVGuardian: address not whitelisted");
+        }
+
+        bool paused = pcvDeposit._paused();
+        if (paused) {
+            pcvDeposit._unpause();
+        }
 
-        if (pauseAfter) {
+        _;
+
+        if (paused) {
             pcvDeposit._pause();
         }
 
         if (depositAfter) {
             IPCVDeposit(safeAddress).deposit();
         }
-
-        emit PCVGuardianERC20Withdrawal(pcvDeposit, safeAddress, token, amount);
     }
 
     // ---------- Internal Functions ----------
 
     function _setSafeAddress(address anAddress) internal {
-        require(safeAddresses.add(anAddress), "set");
+        require(safeAddresses.add(anAddress), "PCVGuardian: already a safe address");
         emit SafeAddressAdded(anAddress);
     }
 
     function _unsetSafeAddress(address anAddress) internal {
-        require(safeAddresses.remove(anAddress), "unset");
+        require(safeAddresses.remove(anAddress), "PCVGuardian: not a safe address");
         emit SafeAddressRemoved(anAddress);
     }
 }