Merge pull request #280 from hopperlabsxyz/fix/pr-270-review-comments

fix: address PR #270 review comments

Author: GuiDupont

src/protocol-v3/OptinProxyFactory.sol

@@ -2,9 +2,8 @@
 pragma solidity 0.8.26;
 
 import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
-import {IERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import {LagoonVault} from "@src/proxy/OptinProxy.sol";
-import {AccessMode} from "@src/v0.6.0/primitives/Enums.sol";
+import {InitStruct} from "@src/v0.6.0/vault/Vault-v0.6.0.sol";
 
 interface IVault {
     function initialize(
@@ -26,42 +25,6 @@ struct OptinProxyFactoryStorage {
     mapping(address => bool) isInstance;
 }
 
-/// @title InitStruct
-/// @notice Initialization structure for creating new vault proxies
-/// @dev Contains all necessary parameters to initialize a vault
-struct InitStruct {
-    /// @notice Underlying ERC20 token for the vault
-    IERC20 underlying;
-    /// @notice Name of the vault token
-    string name;
-    /// @notice Symbol of the vault token
-    string symbol;
-    /// @notice Address of the safe/multisig
-    address safe;
-    /// @notice Address of the whitelist manager
-    address whitelistManager;
-    /// @notice Address of the valuation manager
-    address valuationManager;
-    /// @notice Admin address for the vault
-    address admin;
-    /// @notice Fee receiver address
-    address feeReceiver;
-    /// @notice Management fee rate (in basis points)
-    uint16 managementRate;
-    /// @notice Performance fee rate (in basis points)
-    uint16 performanceRate;
-    /// @notice Access mode (Whitelist or Blacklist)
-    AccessMode accessMode;
-    /// @notice Entry fee rate (in basis points)
-    uint16 entryRate;
-    /// @notice Exit fee rate (in basis points)
-    uint16 exitRate;
-    /// @notice Address of the security council
-    address securityCouncil;
-    /// @notice Address of the external sanctions list
-    address externalSanctionsList;
-}
-
 /// @title OptinProxyFactory
 /// @notice Factory contract for creating and managing OptinProxy instances
 /// @dev Inherits from OwnableUpgradeable to provide ownership functionality
@@ -112,6 +75,7 @@ contract OptinProxyFactory is OwnableUpgradeable {
         $.WRAPPED_NATIVE = _wrappedNativeToken;
     }
 
+    /// @param _logic Address of the vault logic implementation
     /// @param _initialOwner Address of the initial proxy owner
     /// @param _initialDelay The initial delay before which an upgrade can occur by the proxy admin
     /// @param _init Initialization parameters for the vault

src/v0.5.0/Vault.sol

@@ -18,13 +18,13 @@ import {
     ValuationUpdateNotAllowed
 } from "./primitives/Errors.sol";
 
-import {FeeRegistry} from "../protocol-v1/FeeRegistry.sol";
 import {DepositSync, Referral, StateUpdated} from "./primitives/Events.sol";
 import {ERC4626Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC4626Upgradeable.sol";
 import {IERC4626} from "@openzeppelin/contracts/interfaces/IERC4626.sol";
 import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
 import {IERC20, SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
+import {FeeRegistry} from "@src/protocol-v1/FeeRegistry.sol";
 
 using SafeERC20 for IERC20;
 
@@ -59,6 +59,7 @@ struct InitStruct {
 }
 
 /// @custom:contact team@hopperlabs.xyz
+/// @custom:oz-upgrades-from src/v0.4.0/Vault.sol:Vault
 contract Vault is ERC7540, Whitelistable, FeeManager {
     /// @custom:storage-location erc7201:hopper.storage.vault
     /// @param newTotalAssets The new total assets of the vault. It is used to update the totalAssets variable.

src/v0.6.0/FeeManager.sol

@@ -51,14 +51,16 @@ abstract contract FeeManager is Ownable2StepUpgradeable {
     /// @param _decimals the number of decimals of the shares
     /// @param _entryRate the entry fee rate, expressed in BPS
     /// @param _exitRate the exit fee rate, expressed in BPS
+    /// @param _haircutRate the haircut fee rate, expressed in BPS
     // solhint-disable-next-line func-name-mixedcase
     function __FeeManager_init(
         address _registry,
         uint16 _managementRate,
         uint16 _performanceRate,
         uint256 _decimals,
         uint16 _entryRate,
-        uint16 _exitRate
+        uint16 _exitRate,
+        uint16 _haircutRate
     ) internal onlyInitializing {
         FeeManagerStorage storage $ = FeeLib._getFeeManagerStorage();
         FeeLib.updateRates(
@@ -68,7 +70,7 @@ abstract contract FeeManager is Ownable2StepUpgradeable {
                 performanceRate: _performanceRate,
                 entryRate: _entryRate,
                 exitRate: _exitRate,
-                haircutRate: 0 // ????????????
+                haircutRate: _haircutRate
             })
         );
 

src/v0.6.0/GuardRailsManager.sol

@@ -40,7 +40,8 @@ abstract contract GuardrailsManager is Roles {
         });
     }
 
-    // @inheritdoc GuardrailsLib
+    /// @notice Updates the current guardrails policy with a new one.
+    /// @param guardrails_ The new guardrails to be set.
     function updateGuardrails(
         Guardrails calldata guardrails_
     ) external onlySecurityCouncil {

src/v0.6.0/Roles.sol

@@ -2,12 +2,7 @@
 pragma solidity 0.8.26;
 
 import {RolesLib} from "./libraries/RolesLib.sol";
-import {
-    OnlySafe,
-    OnlyValuationManagerOrSecurityCouncil,
-    OnlyWhitelistManager,
-    SafeUpgradeabilityNotAllowed
-} from "./primitives/Errors.sol";
+import {OnlySafe, OnlyWhitelistManager, SafeUpgradeabilityNotAllowed} from "./primitives/Errors.sol";
 import {
     FeeReceiverUpdated,
     SafeUpdated,
@@ -28,8 +23,9 @@ abstract contract Roles is Ownable2StepUpgradeable {
     /// @param safe Every lagoon vault is associated with a Safe smart contract. This address will receive the assets of
     /// the vault and can settle deposits and redeems.
     /// @param feeRegistry The address of the FeeRegistry contract.
-    /// @param valuationManager. This address is responsible of updating the newTotalAssets value of the vault.
-    /// @param owner The address of the owner of the contract. It considered as the admin. It is not visible in the
+    /// @param valuationManager This address is responsible of updating the newTotalAssets value of the vault.
+    /// @param securityCouncil The address of the security council that can update total assets without guardrails.
+    /// @dev owner The address of the owner of the contract. It considered as the admin. It is not visible in the
     /// struct. It can change the others roles and itself. Initiate the fund closing. Disable the whitelist.
     struct RolesStorage {
         address whitelistManager;
@@ -75,8 +71,8 @@ abstract contract Roles is Ownable2StepUpgradeable {
     }
 
     /// @dev Modifier to check if the caller is the valuation manager.
-    modifier onlyValuationManagerOrSecurityCouncil() {
-        RolesLib._onlyValuationManagerOrSecurityCouncil();
+    modifier onlyValuationManager() {
+        RolesLib._onlyValuationManager();
         _;
     }
 

src/v0.6.0/Whitelistable.sol

@@ -10,7 +10,7 @@ import {AccessMode} from "./primitives/Enums.sol";
 abstract contract Whitelistable is Roles {
     /// @custom:storage-definition erc7201:hopper.storage.Whitelistable
     /// @param isWhitelisted The mapping of whitelisted addresses.
-    /// @param whitelistState The current whitelist mode (whitelist or blacklist).
+    /// @param accessMode The current access mode (whitelist or blacklist).
     struct WhitelistableStorage {
         mapping(address => bool) isWhitelisted;
         // in v0.6.0, we replace the bool isActivated with a enum AccessMode

src/v0.6.0/libraries/FeeLib.sol

@@ -25,8 +25,8 @@ library FeeLib {
     uint16 constant MAX_PERFORMANCE_RATE = 5000; // 50 %
     uint16 constant MAX_ENTRY_RATE = 1000; // 10 %
     uint16 constant MAX_EXIT_RATE = 1000; // 10 %
-    uint16 constant MAX_HAIRCUT_RATE = 1000; // 10 %
     uint16 constant MAX_PROTOCOL_RATE = 3000; // 30 %
+    uint16 constant MAX_HAIRCUT_RATE = 1000; // 10 %
 
     // keccak256(abi.encode(uint256(keccak256("hopper.storage.FeeManager")) - 1)) & ~bytes32(uint256(0xff));
     /// @custom:storage-location erc7201:hopper.storage.FeeManager
@@ -46,7 +46,7 @@ library FeeLib {
     /// @param assets the total assets under management
     /// @param annualRate the management rate, expressed in BPS and corresponding to the annual
     /// @param timeElapsed the time elapsed since the last fee calculation in seconds
-    /// @return managementFee the management fee express in assets
+    /// @return managementFee the management fee expressed in assets
     function calculateManagementFee(
         uint256 assets,
         uint256 annualRate,
@@ -227,6 +227,9 @@ library FeeLib {
         if (newRates.exitRate > MAX_EXIT_RATE) {
             revert AboveMaxRate(MAX_EXIT_RATE);
         }
+        if (newRates.haircutRate > MAX_HAIRCUT_RATE) {
+            revert AboveMaxRate(MAX_HAIRCUT_RATE);
+        }
 
         Rates memory currentRates = $.rates;
         $.rates = newRates;

src/v0.6.0/libraries/GuardrailsLib.sol

@@ -14,7 +14,7 @@ library GuardrailsLib {
     // scale to avoid loss of precision
     uint256 public constant SCALE = 1e18;
 
-    /// @custom:storage-definition erc7201:hopper.storage.FeeManager
+    /// @custom:storage-definition erc7201:hopper.storage.GuardrailsManager
     /// @param guardrails The current guardrails.
     struct GuardrailsManagerStorage {
         Guardrails guardrails;
@@ -39,13 +39,11 @@ library GuardrailsLib {
         }
     }
 
-    /**
-     * @notice Checks if a price-per-share (PPS) update is compliant with the current guardrails.
-     * @param currentPps The current price-per-share.
-     * @param nextPps The proposed new price-per-share.
-     * @param _timePast The time elapsed since the last update.
-     * @return bool True if the update is compliant, false otherwise.
-     */
+    /// @notice Checks if a price-per-share (PPS) update is compliant with the current guardrails.
+    /// @param currentPps The current price-per-share.
+    /// @param nextPps The proposed new price-per-share.
+    /// @param _timePast The time elapsed since the last update.
+    /// @return bool True if the update is compliant, false otherwise.
     function isCompliant(
         uint256 currentPps,
         uint256 nextPps,
@@ -87,10 +85,8 @@ library GuardrailsLib {
         }
     }
 
-    /**
-     * @notice Updates the current policy with a new one.
-     * @param guardrails_ The new guardrails to be set.
-     */
+    /// @notice Updates the current policy with a new one.
+    /// @param guardrails_ The new guardrails to be set.
     function updateGuardrails(
         Guardrails memory guardrails_
     ) external {

src/v0.6.0/libraries/RolesLib.sol

@@ -2,13 +2,7 @@
 pragma solidity 0.8.26;
 
 import {Roles} from "../Roles.sol";
-import {OnlySafe, OnlyWhitelistManager} from "../primitives/Errors.sol";
-import {
-    OnlySafe,
-    OnlySecurityCouncil,
-    OnlyValuationManagerOrSecurityCouncil,
-    OnlyWhitelistManager
-} from "../primitives/Errors.sol";
+import {OnlySafe, OnlySecurityCouncil, OnlyValuationManager, OnlyWhitelistManager} from "../primitives/Errors.sol";
 import {
     FeeReceiverUpdated,
     SafeUpdated,
@@ -44,11 +38,10 @@ library RolesLib {
         }
     }
 
-    function _onlyValuationManagerOrSecurityCouncil() internal view {
+    function _onlyValuationManager() internal view {
         address _valuationManager = _getRolesStorage().valuationManager;
-        address _securityCouncil = _getRolesStorage().securityCouncil;
-        if (_valuationManager != msg.sender && _securityCouncil != msg.sender) {
-            revert OnlyValuationManagerOrSecurityCouncil(_valuationManager, _securityCouncil);
+        if (_valuationManager != msg.sender) {
+            revert OnlyValuationManager(_valuationManager);
         }
     }
 

src/v0.6.0/primitives/Errors.sol

@@ -80,10 +80,9 @@ error OnlySafe(address safe);
 /// @param whitelistManager The address of the whitelist manager.
 error OnlyWhitelistManager(address whitelistManager);
 
-/// @notice Indicates that the caller is not the valuation manager or the security council.
+/// @notice Indicates that the caller is not the valuation manager.
 /// @param valuationManager The address of the valuation manager.
-/// @param securityCouncil The address of the security council.
-error OnlyValuationManagerOrSecurityCouncil(address valuationManager, address securityCouncil);
+error OnlyValuationManager(address valuationManager);
 
 /// @notice Indicates that the safe upgradeability has been given up..
 error SafeUpgradeabilityNotAllowed();