[VEN-1795] OZ Diamond Comptroller audit fixes

contracts/Comptroller/ComptrollerStorage.sol

@@ -1,9 +1,11 @@
+// SPDX-License-Identifier: BSD-3-Clause
+
 pragma solidity ^0.5.16;
 
-import "../Tokens/VTokens/VToken.sol";
-import "../Oracle/PriceOracle.sol";
-import "../Tokens/VAI/VAIControllerInterface.sol";
-import "./ComptrollerLensInterface.sol";
+import { VToken } from "../Tokens/VTokens/VToken.sol";
+import { PriceOracle } from "../Oracle/PriceOracle.sol";
+import { VAIControllerInterface } from "../Tokens/VAI/VAIControllerInterface.sol";
+import { ComptrollerLensInterface } from "./ComptrollerLensInterface.sol";
 
 contract UnitrollerAdminStorage {
     /**
@@ -36,17 +38,17 @@ contract ComptrollerV1Storage is UnitrollerAdminStorage {
     /**
      * @notice Multiplier used to calculate the maximum repayAmount when liquidating a borrow
      */
-    uint public closeFactorMantissa;
+    uint256 public closeFactorMantissa;
 
     /**
      * @notice Multiplier representing the discount on collateral that a liquidator receives
      */
-    uint public liquidationIncentiveMantissa;
+    uint256 public liquidationIncentiveMantissa;
 
     /**
      * @notice Max number of assets a single account can participate in (borrow or use as collateral)
      */
-    uint public maxAssets;
+    uint256 public maxAssets;
 
     /**
      * @notice Per-account mapping of "assets you are in", capped by maxAssets
@@ -61,7 +63,7 @@ contract ComptrollerV1Storage is UnitrollerAdminStorage {
          *  For instance, 0.9 to allow borrowing 90% of collateral value.
          *  Must be between 0 and 1, and stored as a mantissa.
          */
-        uint collateralFactorMantissa;
+        uint256 collateralFactorMantissa;
         /// @notice Per-market mapping of "accounts in this asset"
         mapping(address => bool) accountMembership;
         /// @notice Whether or not this market receives XVS
@@ -103,10 +105,10 @@ contract ComptrollerV1Storage is UnitrollerAdminStorage {
     VToken[] public allMarkets;
 
     /// @notice The rate at which the flywheel distributes XVS, per block
-    uint public venusRate;
+    uint256 public venusRate;
 
     /// @notice The portion of venusRate that each market currently receives
-    mapping(address => uint) public venusSpeeds;
+    mapping(address => uint256) public venusSpeeds;
 
     /// @notice The Venus market supply state for each market
     mapping(address => VenusMarketState) public venusSupplyState;
@@ -115,22 +117,22 @@ contract ComptrollerV1Storage is UnitrollerAdminStorage {
     mapping(address => VenusMarketState) public venusBorrowState;
 
     /// @notice The Venus supply index for each market for each supplier as of the last time they accrued XVS
-    mapping(address => mapping(address => uint)) public venusSupplierIndex;
+    mapping(address => mapping(address => uint256)) public venusSupplierIndex;
 
     /// @notice The Venus borrow index for each market for each borrower as of the last time they accrued XVS
-    mapping(address => mapping(address => uint)) public venusBorrowerIndex;
+    mapping(address => mapping(address => uint256)) public venusBorrowerIndex;
 
     /// @notice The XVS accrued but not yet transferred to each user
-    mapping(address => uint) public venusAccrued;
+    mapping(address => uint256) public venusAccrued;
 
     /// @notice The Address of VAIController
     VAIControllerInterface public vaiController;
 
     /// @notice The minted VAI amount to each user
-    mapping(address => uint) public mintedVAIs;
+    mapping(address => uint256) public mintedVAIs;
 
     /// @notice VAI Mint Rate as a percentage
-    uint public vaiMintRate;
+    uint256 public vaiMintRate;
 
     /**
      * @notice The Pause Guardian can pause certain actions as a safety mechanism.
@@ -144,12 +146,12 @@ contract ComptrollerV1Storage is UnitrollerAdminStorage {
     bool public protocolPaused;
 
     /// @notice The rate at which the flywheel distributes XVS to VAI Minters, per block (deprecated)
-    uint private venusVAIRate;
+    uint256 private venusVAIRate;
 }
 
 contract ComptrollerV2Storage is ComptrollerV1Storage {
     /// @notice The rate at which the flywheel distributes XVS to VAI Vault, per block
-    uint public venusVAIVaultRate;
+    uint256 public venusVAIVaultRate;
 
     // address of VAI Vault
     address public vaiVaultAddress;
@@ -166,7 +168,7 @@ contract ComptrollerV3Storage is ComptrollerV2Storage {
     address public borrowCapGuardian;
 
     /// @notice Borrow caps enforced by borrowAllowed for each vToken address. Defaults to zero which corresponds to unlimited borrowing.
-    mapping(address => uint) public borrowCaps;
+    mapping(address => uint256) public borrowCaps;
 }
 
 contract ComptrollerV4Storage is ComptrollerV3Storage {
@@ -182,10 +184,10 @@ contract ComptrollerV4Storage is ComptrollerV3Storage {
 
 contract ComptrollerV5Storage is ComptrollerV4Storage {
     /// @notice The portion of XVS that each contributor receives per block (deprecated)
-    mapping(address => uint) private venusContributorSpeeds;
+    mapping(address => uint256) private venusContributorSpeeds;
 
     /// @notice Last block at which a contributor's XVS rewards have been allocated (deprecated)
-    mapping(address => uint) private lastContributorBlock;
+    mapping(address => uint256) private lastContributorBlock;
 }
 
 contract ComptrollerV6Storage is ComptrollerV5Storage {
@@ -218,15 +220,15 @@ contract ComptrollerV9Storage is ComptrollerV8Storage {
     }
 
     /// @notice True if a certain action is paused on a certain market
-    mapping(address => mapping(uint => bool)) internal _actionPaused;
+    mapping(address => mapping(uint256 => bool)) internal _actionPaused;
 }
 
 contract ComptrollerV10Storage is ComptrollerV9Storage {
     /// @notice The rate at which venus is distributed to the corresponding borrow market (per block)
-    mapping(address => uint) public venusBorrowSpeeds;
+    mapping(address => uint256) public venusBorrowSpeeds;
 
     /// @notice The rate at which venus is distributed to the corresponding supply market (per block)
-    mapping(address => uint) public venusSupplySpeeds;
+    mapping(address => uint256) public venusSupplySpeeds;
 }
 
 contract ComptrollerV11Storage is ComptrollerV10Storage {

contracts/Comptroller/Diamond/Diamond.sol

@@ -1,9 +1,10 @@
+// SPDX-License-Identifier: BSD-3-Clause
+
 pragma solidity 0.5.16;
 pragma experimental ABIEncoderV2;
 
 import { IDiamondCut } from "./interfaces/IDiamondCut.sol";
-import "../ComptrollerStorage.sol";
-import "../Unitroller.sol";
+import { Unitroller, ComptrollerV12Storage } from "../Unitroller.sol";
 
 contract Diamond is ComptrollerV12Storage {
     /// @notice Emitted when functions are added, replaced or removed to facets
@@ -74,7 +75,7 @@ contract Diamond is ComptrollerV12Storage {
      * @return facets_ Array of Facet
      */
     function facets() external view returns (Facet[] memory facets_) {
-        uint facetsLength = _facetAddresses.length;
+        uint256 facetsLength = _facetAddresses.length;
         facets_ = new Facet[](facetsLength);
         for (uint256 i; i < facetsLength; ++i) {
             address facet = _facetAddresses[i];

contracts/Comptroller/Diamond/facets/FacetBase.sol

@@ -1,29 +1,30 @@
+// SPDX-License-Identifier: BSD-3-Clause
+
 pragma solidity 0.5.16;
 
-import "../../../Utils/ErrorReporter.sol";
-import "../../../Tokens/VTokens/VToken.sol";
-import "../../../Utils/ExponentialNoError.sol";
-import "../../../Comptroller/ComptrollerStorage.sol";
-import "../../../Governance/IAccessControlManager.sol";
-import "../../../Utils/SafeBEP20.sol";
+import { VToken, ComptrollerErrorReporter, ExponentialNoError } from "../../../Tokens/VTokens/VToken.sol";
+import { IVAIVault } from "../../../Comptroller/ComptrollerInterface.sol";
+import { ComptrollerV12Storage } from "../../../Comptroller/ComptrollerStorage.sol";
+import { IAccessControlManager } from "../../../Governance/IAccessControlManager.sol";
+import { SafeBEP20, IBEP20 } from "../../../Utils/SafeBEP20.sol";
 
 contract FacetBase is ComptrollerV12Storage, ExponentialNoError {
     /// @notice Emitted when an account enters a market
-    event MarketEntered(VToken vToken, address account);
+    event MarketEntered(VToken indexed vToken, address indexed account);
 
     /// @notice Emitted when XVS is distributed to VAI Vault
-    event DistributedVAIVaultVenus(uint amount);
+    event DistributedVAIVaultVenus(uint256 amount);
 
     using SafeBEP20 for IBEP20;
 
     /// @notice The initial Venus index for a market
-    uint224 public constant venusInitialIndex = 1e36;
+    uint224 public constant VENUS_INITIAL_INDEX = 1e36;
     // closeFactorMantissa must be strictly greater than this value
-    uint internal constant closeFactorMinMantissa = 0.05e18; // 0.05
+    uint256 internal constant CLOSE_FACTOR_MIN_MANTISSA = 0.05e18; // 0.05
     // closeFactorMantissa must not exceed this value
-    uint internal constant closeFactorMaxMantissa = 0.9e18; // 0.9
+    uint256 internal constant CLOSE_FACTOR_MAX_MANTISSA = 0.9e18; // 0.9
     // No collateralFactorMantissa may exceed this value
-    uint internal constant collateralFactorMaxMantissa = 0.9e18; // 0.9
+    uint256 internal constant COLLATERAL_FACTOR_MAX_MANTISSA = 0.9e18; // 0.9
 
     /// @notice Reverts if the protocol is paused
     function checkProtocolPauseState() internal view {
@@ -66,13 +67,13 @@ contract FacetBase is ComptrollerV12Storage, ExponentialNoError {
      * @param market vToken address
      */
     function actionPaused(address market, Action action) public view returns (bool) {
-        return _actionPaused[market][uint(action)];
+        return _actionPaused[market][uint256(action)];
     }
 
     /**
      * @notice Get the latest block number
      */
-    function getBlockNumber() public view returns (uint) {
+    function getBlockNumber() public view returns (uint256) {
         return block.number;
     }
 
@@ -135,10 +136,10 @@ contract FacetBase is ComptrollerV12Storage, ExponentialNoError {
     function getHypotheticalAccountLiquidityInternal(
         address account,
         VToken vTokenModify,
-        uint redeemTokens,
-        uint borrowAmount
-    ) internal view returns (ComptrollerErrorReporter.Error, uint, uint) {
-        (uint err, uint liquidity, uint shortfall) = comptrollerLens.getHypotheticalAccountLiquidity(
+        uint256 redeemTokens,
+        uint256 borrowAmount
+    ) internal view returns (Error, uint256, uint256) {
+        (uint256 err, uint256 liquidity, uint256 shortfall) = comptrollerLens.getHypotheticalAccountLiquidity(
             address(this),
             account,
             vTokenModify,
@@ -182,25 +183,29 @@ contract FacetBase is ComptrollerV12Storage, ExponentialNoError {
      * @param redeemTokens Amount of tokens to redeem
      * @return Success indicator for redeem is allowed or not
      */
-    function redeemAllowedInternal(address vToken, address redeemer, uint redeemTokens) internal view returns (uint) {
+    function redeemAllowedInternal(
+        address vToken,
+        address redeemer,
+        uint256 redeemTokens
+    ) internal view returns (uint256) {
         ensureListed(markets[vToken]);
         /* If the redeemer is not 'in' the market, then we can bypass the liquidity check */
         if (!markets[vToken].accountMembership[redeemer]) {
-            return uint(ComptrollerErrorReporter.Error.NO_ERROR);
+            return uint256(Error.NO_ERROR);
         }
         /* Otherwise, perform a hypothetical liquidity check to guard against shortfall */
-        (ComptrollerErrorReporter.Error err, , uint shortfall) = getHypotheticalAccountLiquidityInternal(
+        (Error err, , uint256 shortfall) = getHypotheticalAccountLiquidityInternal(
             redeemer,
             VToken(vToken),
             redeemTokens,
             0
         );
-        if (err != ComptrollerErrorReporter.Error.NO_ERROR) {
-            return uint(err);
+        if (err != Error.NO_ERROR) {
+            return uint256(err);
         }
         if (shortfall != 0) {
-            return uint(ComptrollerErrorReporter.Error.INSUFFICIENT_LIQUIDITY);
+            return uint256(Error.INSUFFICIENT_LIQUIDITY);
         }
-        return uint(ComptrollerErrorReporter.Error.NO_ERROR);
+        return uint256(Error.NO_ERROR);
     }
 }