[PDE-586] audit remediations 2

nest/src/AggregateToken.sol

@@ -366,7 +366,7 @@ contract AggregateToken is ComponentToken, IAggregateToken, ERC1155Holder {
         uint256 assets,
         uint256 minimumMint,
         address _teller
-    ) public nonReentrant returns (uint256 shares) {
+    ) public nonReentrant onlyRole(MANAGER_ROLE) returns (uint256 shares) {
         if (msg.sender == address(0)) {
             revert InvalidReceiver();
         }

nest/src/ComponentToken.sol

@@ -21,9 +21,20 @@ import { IERC7575 } from "./interfaces/IERC7575.sol";
 
 /**
  * @title ComponentToken
- * @author Eugene Y. Q. Shen
+ * @author Eugene Y. Q. Shen, Alp Guneysel
  * @notice Abstract contract that implements the IComponentToken interface and can be extended
  *   with a concrete implementation that interfaces with an external real-world asset.
+ * @dev Rounding rules for ERC4626 functions:
+ *
+ * Preview functions (simulate what would happen):
+ * - previewMint(shares)     - Round UP ⬆️   // Show max assets needed
+ * - previewWithdraw(assets) - Round UP ⬆️   // Show max shares needed
+ * - previewRedeem(shares)   - Round DOWN ⬇️ // Show min assets to receive
+ * - previewDeposit(assets)  - Round DOWN ⬇️ // Show min shares to receive
+ *
+ * Convert functions (used in actual operations):
+ * - convertToAssets(shares) - Round DOWN ⬇️ // Conservative for withdrawals
+ * - convertToShares(assets) - Round DOWN ⬇️ // Conservative for deposits
  */
 abstract contract ComponentToken is
     Initializable,
@@ -78,6 +89,15 @@ abstract contract ComponentToken is
     /// @notice Base that is used to divide all price inputs in order to represent e.g. 1.000001 as 1000001e12
     uint256 internal constant _BASE = 1e18;
 
+    // Enums
+
+    /// @dev Enum for the parameters that can be zero in the zeroAmount error
+    enum ZeroAmountParam {
+        ASSETS, // Amount of assets in deposit/withdraw operations
+        SHARES // Amount of shares in mint/redeem operations
+
+    }
+
     // Events
 
     /**
@@ -102,7 +122,7 @@ abstract contract ComponentToken is
     error Unimplemented();
 
     /// @notice Indicates a failure because the given amount is 0
-    error ZeroAmount();
+    error ZeroAmount(ZeroAmountParam param);
 
     /**
      * @notice Indicates a failure because the sender is not authorized to perform the action
@@ -279,24 +299,36 @@ abstract contract ComponentToken is
         return super.totalAssets();
     }
 
-    /// @notice Total value held by the given owner
-    /// @dev Reverts with Unimplemented() until convertToAssets is implemented by the concrete contract
-    /// @param owner Address to query the balance of
-    /// @return assets Total value held by the owner
+    /**
+     * @notice Total value held by the given owner
+     * @dev Reverts with Unimplemented() until convertToAssets is implemented by the concrete contract
+     * @param owner Address to query the balance of
+     * @return assets Total value held by the owner
+     */
     function assetsOf(
         address owner
     ) public view virtual returns (uint256 assets) {
         return convertToAssets(balanceOf(owner));
     }
 
-    /// @inheritdoc IERC4626
+    /**
+     * @inheritdoc IERC4626
+     *  @dev Concrete implementations should:
+     *  - Always round DOWN for convertToShares
+     *  - Handle zero supply/assets cases appropriately
+     */
     function convertToShares(
         uint256 assets
     ) public view virtual override(ERC4626Upgradeable, IERC7540) returns (uint256 shares) {
         revert Unimplemented();
     }
 
-    /// @inheritdoc IERC4626
+    /**
+     * @inheritdoc IERC4626
+     * @dev Concrete implementations should:
+     * - Always round DOWN for convertToAssets
+     * - Handle zero supply/assets cases appropriately
+     */
     function convertToAssets(
         uint256 shares
     ) public view virtual override(ERC4626Upgradeable, IERC7540) returns (uint256 assets) {
@@ -312,7 +344,7 @@ abstract contract ComponentToken is
         address owner
     ) public virtual nonReentrant returns (uint256 requestId) {
         if (assets == 0) {
-            revert ZeroAmount();
+            revert ZeroAmount(ZeroAmountParam.ASSETS);
         }
         if (msg.sender != owner) {
             revert Unauthorized(msg.sender, owner);
@@ -338,7 +370,7 @@ abstract contract ComponentToken is
      */
     function _notifyDeposit(uint256 assets, uint256 shares, address controller) internal virtual nonReentrant {
         if (assets == 0) {
-            revert ZeroAmount();
+            revert ZeroAmount(ZeroAmountParam.ASSETS);
         }
 
         ComponentTokenStorage storage $ = _getComponentTokenStorage();
@@ -363,7 +395,7 @@ abstract contract ComponentToken is
         address controller
     ) public virtual nonReentrant returns (uint256 shares) {
         if (assets == 0) {
-            revert ZeroAmount();
+            revert ZeroAmount(ZeroAmountParam.ASSETS);
         }
         if (msg.sender != controller) {
             revert Unauthorized(msg.sender, controller);
@@ -403,7 +435,7 @@ abstract contract ComponentToken is
         address controller
     ) public virtual nonReentrant returns (uint256 assets) {
         if (shares == 0) {
-            revert ZeroAmount();
+            revert ZeroAmount(ZeroAmountParam.SHARES);
         }
         if (msg.sender != controller) {
             revert Unauthorized(msg.sender, controller);
@@ -443,7 +475,7 @@ abstract contract ComponentToken is
         address owner
     ) public virtual nonReentrant returns (uint256 requestId) {
         if (shares == 0) {
-            revert ZeroAmount();
+            revert ZeroAmount(ZeroAmountParam.SHARES);
         }
         if (msg.sender != owner) {
             revert Unauthorized(msg.sender, owner);
@@ -469,7 +501,7 @@ abstract contract ComponentToken is
      */
     function _notifyRedeem(uint256 assets, uint256 shares, address controller) internal virtual nonReentrant {
         if (shares == 0) {
-            revert ZeroAmount();
+            revert ZeroAmount(ZeroAmountParam.SHARES);
         }
 
         ComponentTokenStorage storage $ = _getComponentTokenStorage();
@@ -501,7 +533,7 @@ abstract contract ComponentToken is
         address controller
     ) public virtual override(ERC4626Upgradeable, IERC7540) nonReentrant returns (uint256 assets) {
         if (shares == 0) {
-            revert ZeroAmount();
+            revert ZeroAmount(ZeroAmountParam.SHARES);
         }
         if (msg.sender != controller) {
             revert Unauthorized(msg.sender, controller);
@@ -585,7 +617,7 @@ abstract contract ComponentToken is
         address controller
     ) public virtual override(ERC4626Upgradeable, IERC7540) nonReentrant returns (uint256 shares) {
         if (assets == 0) {
-            revert ZeroAmount();
+            revert ZeroAmount(ZeroAmountParam.ASSETS);
         }
         if (msg.sender != controller) {
             revert Unauthorized(msg.sender, controller);
@@ -695,6 +727,7 @@ abstract contract ComponentToken is
     /**
      * @inheritdoc IERC4626
      * @dev Must revert for all callers and inputs for asynchronous redeem vaults
+     * @dev Round up for previews (show worst case)
      */
     function previewWithdraw(
         uint256 assets

nest/src/NestStaking.sol

@@ -23,8 +23,10 @@ contract NestStaking is Initializable, AccessControlUpgradeable, UUPSUpgradeable
     struct NestStakingStorage {
         /// @dev List of featured AggregateTokens
         IAggregateToken[] featuredList;
-        /// @dev Mapping of featured AggregateTokens
-        mapping(IAggregateToken aggregateToken => bool featured) isFeatured;
+        /// @dev Mapping of AggregateToken to its position in featuredList (1-based indexing)
+        /// @dev Returns 0 if token is not featured, otherwise returns index + 1
+        /// @dev Example: If token is at index 2, featuredIndex[token] = 3
+        mapping(IAggregateToken aggregateToken => uint256 index) featuredIndex;
     }
 
     // keccak256(abi.encode(uint256(keccak256("plume.storage.NestStaking")) - 1)) & ~bytes32(uint256(0xff))
@@ -65,6 +67,19 @@ contract NestStaking is Initializable, AccessControlUpgradeable, UUPSUpgradeable
      */
     event TokenUnfeatured(IAggregateToken aggregateToken);
 
+    // Enums
+    enum ZeroAmountParam {
+        ASK_PRICE, // Price at which users can buy tokens
+        BID_PRICE, // Price at which users can sell tokens
+        INITIAL_SUPPLY, // Initial supply of tokens when creating
+        TOTAL_VALUE, // Total value of all tokens
+        MINT_AMOUNT, // Amount of tokens to mint
+        BURN_AMOUNT, // Amount of tokens to burn
+        DEPOSIT_AMOUNT, // Amount of tokens to deposit
+        REDEEM_AMOUNT // Amount of tokens to redeem
+
+    }
+
     // Errors
 
     /**
@@ -79,6 +94,30 @@ contract NestStaking is Initializable, AccessControlUpgradeable, UUPSUpgradeable
      */
     error TokenNotFeatured(IAggregateToken aggregateToken);
 
+    /**
+     * @notice Indicates a failure because there are no featured tokens
+     */
+    error NoFeaturedTokens();
+
+    /**
+     * @notice Indicates a failure because the given address is zero
+     * @param what Description of which address parameter was zero
+     */
+    error ZeroAddress(string what);
+
+    /**
+     * @notice Indicates a failure because the given amount is zero
+     * @param param Description of which amount parameter was zero
+     */
+    error ZeroAmount(ZeroAmountParam param);
+
+    /**
+     * @notice Indicates a failure because bid price is greater than ask price
+     * @param bidPrice Price at which users can sell the token
+     * @param askPrice Price at which users can buy the token
+     */
+    error InvalidPrices(uint256 bidPrice, uint256 askPrice);
+
     // Initializer
 
     /**
@@ -124,11 +163,18 @@ contract NestStaking is Initializable, AccessControlUpgradeable, UUPSUpgradeable
         IAggregateToken aggregateToken
     ) external onlyRole(ADMIN_ROLE) {
         NestStakingStorage storage $ = _getNestStakingStorage();
-        if ($.isFeatured[aggregateToken]) {
+
+        // Add zero address check
+        if (address(aggregateToken) == address(0)) {
+            revert ZeroAddress("aggregateToken");
+        }
+
+        if ($.featuredIndex[aggregateToken] != 0) {
             revert TokenAlreadyFeatured(aggregateToken);
         }
         $.featuredList.push(aggregateToken);
-        $.isFeatured[aggregateToken] = true;
+        // Store index + 1 (so 0 means not featured)
+        $.featuredIndex[aggregateToken] = $.featuredList.length;
         emit TokenFeatured(aggregateToken);
     }
 
@@ -141,19 +187,33 @@ contract NestStaking is Initializable, AccessControlUpgradeable, UUPSUpgradeable
         IAggregateToken aggregateToken
     ) external onlyRole(ADMIN_ROLE) {
         NestStakingStorage storage $ = _getNestStakingStorage();
-        if (!$.isFeatured[aggregateToken]) {
+
+        // Check if there are any featured tokens
+        if ($.featuredList.length == 0) {
+            revert NoFeaturedTokens();
+        }
+
+        // Get stored index (subtract 1 to get actual index)
+        uint256 storedIndex = $.featuredIndex[aggregateToken];
+        if (storedIndex == 0) {
             revert TokenNotFeatured(aggregateToken);
         }
-        IAggregateToken[] storage featuredList = $.featuredList;
-        uint256 length = featuredList.length;
-        for (uint256 i = 0; i < length; ++i) {
-            if (featuredList[i] == aggregateToken) {
-                featuredList[i] = featuredList[length - 1];
-                featuredList.pop();
-                break;
-            }
+        uint256 index = storedIndex - 1;
+
+        // Get the last token
+        uint256 lastIndex = $.featuredList.length - 1;
+        IAggregateToken lastToken = $.featuredList[lastIndex];
+
+        // Move last token to the removed position (unless it's the last position)
+        if (index != lastIndex) {
+            $.featuredList[index] = lastToken;
+            $.featuredIndex[lastToken] = storedIndex; // Update index of moved token
         }
-        $.isFeatured[aggregateToken] = false;
+
+        // Remove last element and clear mapping
+        $.featuredList.pop();
+        $.featuredIndex[aggregateToken] = 0;
+
         emit TokenUnfeatured(aggregateToken);
     }
 
@@ -183,6 +243,23 @@ contract NestStaking is Initializable, AccessControlUpgradeable, UUPSUpgradeable
     ) public returns (IAggregateToken aggregateToken) {
         NestStakingStorage storage $ = _getNestStakingStorage();
 
+        // Input validations
+        if (owner == address(0)) {
+            revert ZeroAddress("owner");
+        }
+        if (address(currencyToken) == address(0)) {
+            revert ZeroAddress("currencyToken");
+        }
+        if (askPrice == 0) {
+            revert ZeroAmount(ZeroAmountParam.ASK_PRICE);
+        }
+        if (bidPrice == 0) {
+            revert ZeroAmount(ZeroAmountParam.BID_PRICE);
+        }
+        if (bidPrice > askPrice) {
+            revert InvalidPrices(bidPrice, askPrice); // Need to add this error
+        }
+
         IAggregateToken aggregateTokenImplementation = new AggregateToken();
         AggregateTokenProxy aggregateTokenProxy = new AggregateTokenProxy(
             address(aggregateTokenImplementation),
@@ -191,7 +268,8 @@ contract NestStaking is Initializable, AccessControlUpgradeable, UUPSUpgradeable
 
         aggregateToken = IAggregateToken(address(aggregateTokenProxy));
         $.featuredList.push(aggregateToken);
-        $.isFeatured[aggregateToken] = true;
+        $.featuredIndex[aggregateToken] = $.featuredList.length;
+
         emit TokenFeatured(aggregateToken);
         emit TokenCreated(msg.sender, aggregateToken);
     }
@@ -211,7 +289,7 @@ contract NestStaking is Initializable, AccessControlUpgradeable, UUPSUpgradeable
     function isFeatured(
         IAggregateToken aggregateToken
     ) external view returns (bool featured) {
-        return _getNestStakingStorage().isFeatured[aggregateToken];
+        return _getNestStakingStorage().featuredIndex[aggregateToken] != 0;
     }
 
 }