feat: PR feedback + new tests for GhoStewardV2

src/contracts/misc/GhoStewardV2.sol

@@ -33,13 +33,13 @@ contract GhoStewardV2 is Ownable, IGhoStewardV2 {
   using EnumerableSet for EnumerableSet.AddressSet;
 
   /// @inheritdoc IGhoStewardV2
-  uint256 public constant GHO_BORROW_RATE_CHANGE_MAX = 0.005e27;
+  uint256 public constant GHO_BORROW_RATE_CHANGE_MAX = 0.0050e27; // 0.5%
 
   /// @inheritdoc IGhoStewardV2
-  uint256 public constant GSM_FEE_RATE_CHANGE_MAX = 0.005e4;
+  uint256 public constant GSM_FEE_RATE_CHANGE_MAX = 0.0050e4; // 0.5%
 
   /// @inheritdoc IGhoStewardV2
-  uint256 public constant GHO_BORROW_RATE_MAX = 9.5e27;
+  uint256 public constant GHO_BORROW_RATE_MAX = 0.095e27; // 9.5%
 
   /// @inheritdoc IGhoStewardV2
   uint256 public constant MINIMUM_DELAY = 7 days;
@@ -106,11 +106,7 @@ contract GhoStewardV2 is Ownable, IGhoStewardV2 {
     require(_controlledFacilitatorsByAddress[facilitator], 'FACILITATOR_NOT_IN_CONTROL');
     (uint256 currentBucketCapacity, ) = IGhoToken(GHO_TOKEN).getFacilitatorBucket(facilitator);
     require(
-      _isChangePositiveAndIncreaseLowerThanMax(
-        currentBucketCapacity,
-        newBucketCapacity,
-        currentBucketCapacity
-      ),
+      _isIncreaseLowerThanMax(currentBucketCapacity, newBucketCapacity, currentBucketCapacity),
       'INVALID_BUCKET_CAPACITY_UPDATE'
     );
 
@@ -134,13 +130,10 @@ contract GhoStewardV2 is Ownable, IGhoStewardV2 {
     uint256 currentBorrowRate = GhoInterestRateStrategy(ghoReserveData.interestRateStrategyAddress)
       .getBaseVariableBorrowRate();
     require(
-      _isChangePositiveAndIncreaseLowerThanMax(
-        currentBorrowRate,
-        newBorrowRate,
-        GHO_BORROW_RATE_CHANGE_MAX
-      ) && newBorrowRate <= GHO_BORROW_RATE_MAX,
+      _isIncreaseLowerThanMax(currentBorrowRate, newBorrowRate, GHO_BORROW_RATE_CHANGE_MAX),
       'INVALID_BORROW_RATE_UPDATE'
     );
+    require(newBorrowRate <= GHO_BORROW_RATE_MAX, 'INVALID_BORROW_RATE_UPDATE');
     address cachedStrategyAddress = _ghoBorrowRateStrategiesByRate[newBorrowRate];
 
     if (cachedStrategyAddress == address(0)) {
@@ -167,11 +160,7 @@ contract GhoStewardV2 is Ownable, IGhoStewardV2 {
   ) external onlyRiskCouncil notLocked(_gsmTimelocksByAddress[gsm].gsmExposureCapLastUpdated) {
     uint128 currentExposureCap = IGsm(gsm).getExposureCap();
     require(
-      _isChangePositiveAndIncreaseLowerThanMax(
-        currentExposureCap,
-        newExposureCap,
-        currentExposureCap
-      ),
+      _isIncreaseLowerThanMax(currentExposureCap, newExposureCap, currentExposureCap),
       'INVALID_EXPOSURE_CAP_UPDATE'
     );
     _gsmTimelocksByAddress[gsm].gsmExposureCapLastUpdated = uint40(block.timestamp);
@@ -186,11 +175,11 @@ contract GhoStewardV2 is Ownable, IGhoStewardV2 {
   ) external onlyRiskCouncil notLocked(_gsmTimelocksByAddress[gsm].gsmFeeStrategyLastUpdated) {
     address currentFeeStrategy = IGsm(gsm).getFeeStrategy();
     require(currentFeeStrategy != address(0), 'GSM_FEE_STRATEGY_NOT_FOUND');
-    uint256 currentBuyFee = IGsmFeeStrategy(gsm).getBuyFee(10e5);
-    uint256 currentSellFee = IGsmFeeStrategy(gsm).getSellFee(10e5);
+    uint256 currentBuyFee = IGsmFeeStrategy(currentFeeStrategy).getBuyFee(1e4);
+    uint256 currentSellFee = IGsmFeeStrategy(currentFeeStrategy).getSellFee(1e4);
     require(
-      _isChangePositiveAndIncreaseLowerThanMax(currentBuyFee, buyFee, GSM_FEE_RATE_CHANGE_MAX) &&
-        _isChangePositiveAndIncreaseLowerThanMax(currentSellFee, sellFee, GSM_FEE_RATE_CHANGE_MAX),
+      _isIncreaseLowerThanMax(currentBuyFee, buyFee, GSM_FEE_RATE_CHANGE_MAX) &&
+        _isIncreaseLowerThanMax(currentSellFee, sellFee, GSM_FEE_RATE_CHANGE_MAX),
       'INVALID_FEE_STRATEGY_UPDATE'
     );
     address cachedStrategyAddress = _gsmFeeStrategiesByRates[buyFee][sellFee];
@@ -205,14 +194,13 @@ contract GhoStewardV2 is Ownable, IGhoStewardV2 {
   }
 
   /// @inheritdoc IGhoStewardV2
-  function controlFacilitators(address[] memory facilitatorList, bool approve) external onlyOwner {
+  function setControlledFacilitator(
+    address[] memory facilitatorList,
+    bool approve
+  ) external onlyOwner {
     for (uint256 i = 0; i < facilitatorList.length; i++) {
       _controlledFacilitatorsByAddress[facilitatorList[i]] = approve;
       if (approve) {
-        IGhoToken.Facilitator memory facilitator = IGhoToken(GHO_TOKEN).getFacilitator(
-          facilitatorList[i]
-        );
-        require(bytes(facilitator.label).length > 0, 'FACILITATOR_DOES_NOT_EXIST');
         _controlledFacilitators.add(facilitatorList[i]);
       } else {
         _controlledFacilitators.remove(facilitatorList[i]);
@@ -253,12 +241,13 @@ contract GhoStewardV2 is Ownable, IGhoStewardV2 {
   }
 
   /**
-   * @notice Ensures the borrow rate change is within the allowed range and is smaller than the maximum allowed.
-   * @param from current borrow rate (in ray)
-   * @param to new borrow rate (in ray)
-   * @return bool true, if difference is within the max 1% change window
+   * @notice Ensures that the change is positive and the difference is lower than max.
+   * @param from current value
+   * @param to new value
+   * @param max maximum difference between from and to
+   * @return bool true, if difference if change is possitive and lower than max
    */
-  function _isChangePositiveAndIncreaseLowerThanMax(
+  function _isIncreaseLowerThanMax(
     uint256 from,
     uint256 to,
     uint256 max

src/contracts/misc/interfaces/IGhoStewardV2.sol

@@ -14,13 +14,13 @@ interface IGhoStewardV2 {
 
   /**
    * @notice Returns the maximum increase for GHO borrow rate updates.
-   * @return The maximum increase change for borrow rate updates in ray (e.g. 0.01e27 results in 1.0%)
+   * @return The maximum increase change for borrow rate updates in ray (e.g. 0.010e27 results in 1.00%)
    */
   function GHO_BORROW_RATE_CHANGE_MAX() external view returns (uint256);
 
   /**
    * @notice Returns the maximum increase for GSM fee rates (buy or sell).
-   * @return The maximum increase change for GSM fee rates updates in bps (e.g. 0.01e4 results in 1.0%)
+   * @return The maximum increase change for GSM fee rates updates in bps (e.g. 0.010e4 results in 1.00%)
    */
   function GSM_FEE_RATE_CHANGE_MAX() external view returns (uint256);
 
@@ -56,7 +56,7 @@ interface IGhoStewardV2 {
 
   /**
    * @notice Updates the bucket capacity of facilitator, only if:
-   * - respects the debounce duration (7 day pause between updates must be respected)
+   * - respects `MINIMUM_DELAY`, the minimum time delay between updates
    * - the update changes up to 100% upwards
    * - the facilitator is controlled
    * @dev Only callable by Risk Council
@@ -67,9 +67,9 @@ interface IGhoStewardV2 {
 
   /**
    * @notice Updates the borrow rate of GHO, only if:
-   * - respects the debounce duration (7 day pause between updates must be respected)
-   * - the update changes up to 0.5% upwards
-   * - the update is lower than 9.5%
+   * - respects `MINIMUM_DELAY`, the minimum time delay between updates
+   * - the update changes up to `GHO_BORROW_RATE_CHANGE_MAX` upwards
+   * - the update is lower than `GHO_BORROW_RATE_MAX`
    * @dev Only callable by Risk Council
    * @param newBorrowRate The new variable borrow rate (expressed in ray) (e.g. 0.0150e27 results in 1.50%)
    */
@@ -88,9 +88,9 @@ interface IGhoStewardV2 {
 
   /**
    * @notice Updates the borrow rate of GHO, only if:
-   * - respects the debounce duration (7 day pause between updates must be respected)
+   * - respects `MINIMUM_DELAY`, the minimum time delay between updates
    * - the GSM address is approved
-   * - the update changes up to 50 bps (0.5%) upwards (for both buy and sell individually);
+   * - the update changes up to `GSM_FEE_RATE_CHANGE_MAX` upwards (for both buy and sell individually);
    * @dev Only callable by Risk Council
    * @param gsm The gsm address to update
    * @param buyFee The new buy fee (expressed in bps) (e.g. 0.0150e4 results in 1.50%)
@@ -104,7 +104,7 @@ interface IGhoStewardV2 {
    * @param facilitatorList A list of facilitators addresses to add to control
    * @param approve A boolean to control or remove control towards the facilitators
    */
-  function controlFacilitators(address[] memory facilitatorList, bool approve) external;
+  function setControlledFacilitator(address[] memory facilitatorList, bool approve) external;
 
   /**
    * @notice Returns the list of controlled facilitators.
@@ -113,7 +113,7 @@ interface IGhoStewardV2 {
   function getControlledFacilitators() external view returns (address[] memory);
 
   /**
-   * @notice Returns the GHO timelock value for borrow rate updates
+   * @notice Returns timestamp of the last GHO borrow rate update
    * @return The time of the last GHO borrow rate update (in seconds).
    */
   function getGhoBorrowRateTimelock() external view returns (uint40);
@@ -123,11 +123,10 @@ interface IGhoStewardV2 {
    * @param gsm The GSM address
    * @return The GsmDebounce struct with parameters' timelock
    */
-
   function getGsmTimelocks(address gsm) external view returns (GsmDebounce memory);
 
   /**
-   * @notice Returns the facilitator timelock value for bucket capacity updates
+   * @notice Returns timestamp of the facilitators last bucket capacity update
    * @param facilitator The facilitator address
    * @return The time of the last bucket capacity (in seconds).
    */

src/test/TestGhoBase.t.sol

@@ -304,7 +304,7 @@ contract TestGhoBase is Test, Constants, Events {
     controlledFacilitators[0] = address(GHO_ATOKEN);
     controlledFacilitators[1] = address(GHO_GSM);
     vm.prank(SHORT_EXECUTOR);
-    GHO_STEWARD_V2.controlFacilitators(controlledFacilitators, true);
+    GHO_STEWARD_V2.setControlledFacilitator(controlledFacilitators, true);
   }
 
   function ghoFaucet(address to, uint256 amount) public {