OZ Audit changes for OETH Withdrawal Queue

contracts/contracts/vault/OETHVaultCore.sol

@@ -34,7 +34,7 @@ contract OETHVaultCore is VaultCore {
      */
     function cacheWETHAssetIndex() external onlyGovernor {
         uint256 assetCount = allAssets.length;
-        for (uint256 i = 0; i < assetCount; ++i) {
+        for (uint256 i; i < assetCount; ++i) {
             if (allAssets[i] == weth) {
                 wethAssetIndex = i;
                 break;
@@ -157,12 +157,12 @@ contract OETHVaultCore is VaultCore {
      * The OETH is burned on request and the WETH is transferred to the withdrawer on claim.
      * This request can be claimed once the withdrawal queue's `claimable` amount
      * is greater than or equal this request's `queued` amount.
-     * There is no minimum time or block number before a request can be claimed. It just needs
+     * There is a minimum of 10 minutes before a request can be claimed. After that, the request just needs
      * enough WETH liquidity in the Vault to satisfy all the outstanding requests to that point in the queue.
      * OETH is converted to WETH at 1:1.
      * @param _amount Amount of OETH to burn.
-     * @param requestId Unique ID for the withdrawal request
-     * @param queued Cumulative total of all WETH queued including already claimed requests.
+     * @return requestId Unique ID for the withdrawal request
+     * @return queued Cumulative total of all WETH queued including already claimed requests.
      */
     function requestWithdrawal(uint256 _amount)
         external
@@ -255,7 +255,7 @@ contract OETHVaultCore is VaultCore {
      * @return amounts Amount of WETH received for each request
      * @return totalAmount Total amount of WETH transferred to the withdrawer
      */
-    function claimWithdrawals(uint256[] memory _requestIds)
+    function claimWithdrawals(uint256[] calldata _requestIds)
         external
         virtual
         whenNotCapitalPaused
@@ -272,7 +272,7 @@ contract OETHVaultCore is VaultCore {
         _addWithdrawalQueueLiquidity();
 
         amounts = new uint256[](_requestIds.length);
-        for (uint256 i = 0; i < _requestIds.length; ++i) {
+        for (uint256 i; i < _requestIds.length; ++i) {
             amounts[i] = _claimWithdrawal(_requestIds[i]);
             totalAmount += amounts[i];
         }
@@ -391,7 +391,7 @@ contract OETHVaultCore is VaultCore {
 
     /// @dev Get the balance of an asset held in Vault and all strategies
     /// less any WETH that is reserved for the withdrawal queue.
-    /// This will only return a non-zero balance for WETH.
+    /// WETH is the only asset that can return a non-zero balance.
     /// All other assets will return 0 even if there is some dust amounts left in the Vault.
     /// For example, there is 1 wei left of stETH in the OETH Vault but will return 0 in this function.
     ///

contracts/contracts/vault/VaultAdmin.sol

@@ -219,7 +219,7 @@ contract VaultAdmin is VaultStorage {
         bytes calldata _data
     ) internal virtual returns (uint256 toAssetAmount) {
         // Check fromAsset and toAsset are valid
-        Asset memory fromAssetConfig = assets[address(_fromAsset)];
+        Asset memory fromAssetConfig = assets[_fromAsset];
         Asset memory toAssetConfig = assets[_toAsset];
         require(fromAssetConfig.isSupported, "From asset is not supported");
         require(toAssetConfig.isSupported, "To asset is not supported");

contracts/contracts/vault/VaultCore.sol

@@ -23,11 +23,8 @@ import "./VaultInitializer.sol";
 contract VaultCore is VaultInitializer {
     using SafeERC20 for IERC20;
     using StableMath for uint256;
-    // max signed int
+    /// @dev max signed int
     uint256 internal constant MAX_INT = 2**255 - 1;
-    // max un-signed int
-    uint256 internal constant MAX_UINT =
-        0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff;
 
     /**
      * @dev Verifies that the rebasing is not paused.
@@ -45,6 +42,9 @@ contract VaultCore is VaultInitializer {
         _;
     }
 
+    /**
+     * @dev Verifies that the caller is the AMO strategy.
+     */
     modifier onlyOusdMetaStrategy() {
         require(
             msg.sender == ousdMetaStrategy,
@@ -67,6 +67,12 @@ contract VaultCore is VaultInitializer {
         _mint(_asset, _amount, _minimumOusdAmount);
     }
 
+    /**
+     * @dev Deposit a supported asset and mint OTokens.
+     * @param _asset Address of the asset being deposited
+     * @param _amount Amount of the asset being deposited
+     * @param _minimumOusdAmount Minimum OTokens to mint
+     */
     function _mint(
         address _asset,
         uint256 _amount,
@@ -433,7 +439,7 @@ contract VaultCore is VaultInitializer {
         returns (uint256 value)
     {
         uint256 assetCount = allAssets.length;
-        for (uint256 y = 0; y < assetCount; ++y) {
+        for (uint256 y; y < assetCount; ++y) {
             address assetAddr = allAssets[y];
             uint256 balance = IERC20(assetAddr).balanceOf(address(this));
             if (balance > 0) {
@@ -465,7 +471,7 @@ contract VaultCore is VaultInitializer {
     {
         IStrategy strategy = IStrategy(_strategyAddr);
         uint256 assetCount = allAssets.length;
-        for (uint256 y = 0; y < assetCount; ++y) {
+        for (uint256 y; y < assetCount; ++y) {
             address assetAddr = allAssets[y];
             if (strategy.supportsAsset(assetAddr)) {
                 uint256 balance = strategy.checkBalance(assetAddr);
@@ -733,6 +739,11 @@ contract VaultCore is VaultInitializer {
         }
     }
 
+    /**
+     * @dev Get the number of decimals of a token asset
+     * @param _asset Address of the asset
+     * @return decimals number of decimals
+     */
     function _getDecimals(address _asset)
         internal
         view
@@ -751,6 +762,7 @@ contract VaultCore is VaultInitializer {
 
     /**
      * @notice Gets the vault configuration of a supported asset.
+     * @param _asset Address of the token asset
      */
     function getAssetConfig(address _asset)
         public

contracts/contracts/vault/VaultStorage.sol

@@ -126,14 +126,15 @@ contract VaultStorage is Initializable, Governable {
     // slither-disable-next-line uninitialized-state
     OUSD internal oUSD;
 
-    //keccak256("OUSD.vault.governor.admin.impl");
+    /// @dev Storage slot for the address of the VaultAdmin contract that is delegated to
+    // keccak256("OUSD.vault.governor.admin.impl");
     bytes32 constant adminImplPosition =
         0xa2bd3d3cf188a41358c8b401076eb59066b09dec5775650c0de4c55187d17bd9;
 
-    // Address of the contract responsible for post rebase syncs with AMMs
+    /// @dev Address of the contract responsible for post rebase syncs with AMMs
     address private _deprecated_rebaseHooksAddr = address(0);
 
-    // Deprecated: Address of Uniswap
+    /// @dev Deprecated: Address of Uniswap
     // slither-disable-next-line constable-states
     address private _deprecated_uniswapAddr = address(0);