Merge pull request #34 from Ion-Protocol/hrikb/N-03

N-03 (OZ Audit)

script/08_DeployInitialHandlers.s.sol

@@ -68,26 +68,23 @@ contract DeployInitialHandlersScript is BaseScript {
                 ionPool,
                 wstEthGemJoin,
                 whitelist,
-                FACTORY,
-                WSTETH_WETH_POOL,
-                WSTETH_WETH_POOL_FEE
+                WSTETH_WETH_POOL
             );
         ethXHandler = new EthXHandler(
                 ETHX_ILK_INDEX,
                 ionPool,
                 ethXGemJoin,
                 MAINNET_STADER,
                 whitelist,
-                WSTETH_WETH_POOL
+                WSTETH_WETH_POOL,
+                0x37b18b10ce5635a84834b26095a0ae5639dcb7520000000000000000000005cb
             );
         swEthHandler = new SwEthHandler(
                 SWETH_ILK_INDEX,
                 ionPool,
                 swEthGemJoin,
                 whitelist,
-                FACTORY,
-                SWETH_ETH_POOL,
-                SWETH_ETH_POOL_FEE
+                SWETH_ETH_POOL
             );
     }
 }

src/InterestRate.sol

@@ -76,36 +76,35 @@ contract InterestRate {
 
     error CollateralIndexOutOfBounds();
     error DistributionFactorsDoNotSumToOne(uint256 sum);
-    error TotalDebtsLength(uint256 COLLATERAL_COUNT, uint256 totalIlkDebtsLength);
     error InvalidYieldOracleAddress();
 
     /**
      * @dev Packed collateral configs
      */
-    uint256 internal immutable ILKCONFIG_0A;
-    uint256 internal immutable ILKCONFIG_0B;
-    uint256 internal immutable ILKCONFIG_0C;
-    uint256 internal immutable ILKCONFIG_1A;
-    uint256 internal immutable ILKCONFIG_1B;
-    uint256 internal immutable ILKCONFIG_1C;
-    uint256 internal immutable ILKCONFIG_2A;
-    uint256 internal immutable ILKCONFIG_2B;
-    uint256 internal immutable ILKCONFIG_2C;
-    uint256 internal immutable ILKCONFIG_3A;
-    uint256 internal immutable ILKCONFIG_3B;
-    uint256 internal immutable ILKCONFIG_3C;
-    uint256 internal immutable ILKCONFIG_4A;
-    uint256 internal immutable ILKCONFIG_4B;
-    uint256 internal immutable ILKCONFIG_4C;
-    uint256 internal immutable ILKCONFIG_5A;
-    uint256 internal immutable ILKCONFIG_5B;
-    uint256 internal immutable ILKCONFIG_5C;
-    uint256 internal immutable ILKCONFIG_6A;
-    uint256 internal immutable ILKCONFIG_6B;
-    uint256 internal immutable ILKCONFIG_6C;
-    uint256 internal immutable ILKCONFIG_7A;
-    uint256 internal immutable ILKCONFIG_7B;
-    uint256 internal immutable ILKCONFIG_7C;
+    uint256 private immutable ILKCONFIG_0A;
+    uint256 private immutable ILKCONFIG_0B;
+    uint256 private immutable ILKCONFIG_0C;
+    uint256 private immutable ILKCONFIG_1A;
+    uint256 private immutable ILKCONFIG_1B;
+    uint256 private immutable ILKCONFIG_1C;
+    uint256 private immutable ILKCONFIG_2A;
+    uint256 private immutable ILKCONFIG_2B;
+    uint256 private immutable ILKCONFIG_2C;
+    uint256 private immutable ILKCONFIG_3A;
+    uint256 private immutable ILKCONFIG_3B;
+    uint256 private immutable ILKCONFIG_3C;
+    uint256 private immutable ILKCONFIG_4A;
+    uint256 private immutable ILKCONFIG_4B;
+    uint256 private immutable ILKCONFIG_4C;
+    uint256 private immutable ILKCONFIG_5A;
+    uint256 private immutable ILKCONFIG_5B;
+    uint256 private immutable ILKCONFIG_5C;
+    uint256 private immutable ILKCONFIG_6A;
+    uint256 private immutable ILKCONFIG_6B;
+    uint256 private immutable ILKCONFIG_6C;
+    uint256 private immutable ILKCONFIG_7A;
+    uint256 private immutable ILKCONFIG_7B;
+    uint256 private immutable ILKCONFIG_7C;
 
     uint256 public immutable COLLATERAL_COUNT;
     IYieldOracle public immutable YIELD_ORACLE;
@@ -140,7 +139,7 @@ contract InterestRate {
         IlkData[] memory ilkDataList,
         uint256 index
     )
-        internal
+        private
         view
         returns (uint256 packedConfig_a, uint256 packedConfig_b, uint256 packedConfig_c)
     {

src/IonPool.sol

@@ -31,15 +31,14 @@ contract IonPool is IonPausableUpgradeable, RewardModule {
     error TakingWethWithoutConsent(address payer, address unconsentedOperator);
     error VaultCannotBeDusty(uint256 amountLeft, uint256 dust);
     error ArithmeticError();
-    error SpotUpdaterNotAuthorized();
     error IlkAlreadyAdded(address ilkAddress);
     error IlkNotInitialized(uint256 ilkIndex);
     error DepositSurpassesSupplyCap(uint256 depositAmount, uint256 supplyCap);
     error MaxIlksReached();
 
     error InvalidIlkAddress();
     error InvalidInterestRateModule(InterestRate invalidInterestRateModule);
-    error InvalidWhitelist(Whitelist invalidWhitelist);
+    error InvalidWhitelist();
 
     // --- Events ---
     event IlkInitialized(uint8 indexed ilkIndex, address indexed ilkAddress);
@@ -50,8 +49,8 @@ contract IonPool is IonPausableUpgradeable, RewardModule {
     event InterestRateModuleUpdated(address newModule);
     event WhitelistUpdated(address newWhitelist);
 
-    event AddOperator(address indexed from, address indexed to);
-    event RemoveOperator(address indexed from, address indexed to);
+    event AddOperator(address indexed user, address indexed operator);
+    event RemoveOperator(address indexed user, address indexed operator);
     event MintAndBurnGem(uint8 indexed ilkIndex, address indexed usr, int256 wad);
     event TransferGem(uint8 indexed ilkIndex, address indexed src, address indexed dst, uint256 wad);
 
@@ -123,14 +122,15 @@ contract IonPool is IonPausableUpgradeable, RewardModule {
         uint256 normalizedDebt; // Normalised Debt    [WAD]
     }
 
+    /// @custom:storage-location erc7201:ion.storage.IonPool
     struct IonPoolStorage {
         Ilk[] ilks;
         // remove() should never be called, it will mess up the ordering
         EnumerableSet.AddressSet ilkAddresses;
         mapping(uint256 ilkIndex => mapping(address user => Vault)) vaults;
         mapping(uint256 ilkIndex => mapping(address user => uint256)) gem; // [WAD]
-        mapping(address => uint256) unbackedDebt; // [RAD]
-        mapping(address => mapping(address => uint256)) isOperator;
+        mapping(address unbackedDebtor => uint256) unbackedDebt; // [RAD]
+        mapping(address user => mapping(address operator => uint256)) isOperator;
         uint256 debt; // Total Debt [RAD]
         uint256 weth; // liquidity in pool [WAD]
         uint256 wethSupplyCap; // [WAD]
@@ -206,7 +206,7 @@ contract IonPool is IonPausableUpgradeable, RewardModule {
         $.ilks.push(newIlk);
         Ilk storage ilk = $.ilks[ilkIndex];
 
-        ilk.rate = 10 ** 27;
+        ilk.rate = uint104(RAY);
         // Unsafe cast OK
         ilk.lastRateUpdate = uint48(block.timestamp);
 
@@ -299,7 +299,7 @@ contract IonPool is IonPausableUpgradeable, RewardModule {
      * @param _whitelist new whitelist address.
      */
     function updateWhitelist(Whitelist _whitelist) external onlyRole(ION) {
-        if (address(_whitelist) == address(0)) revert InvalidWhitelist(_whitelist);
+        if (address(_whitelist) == address(0)) revert InvalidWhitelist();
 
         IonPoolStorage storage $ = _getIonPoolStorage();
 
@@ -367,7 +367,7 @@ contract IonPool is IonPausableUpgradeable, RewardModule {
      * @dev Updates accumulators for all `ilk`s based on current interest rates.
      * @return newTotalDebt the new total debt after interest accrual
      */
-    function accrueInterest() external whenNotPaused(Pauses.SAFE) returns (uint256 newTotalDebt) {
+    function accrueInterest() external whenNotPaused(Pauses.SAFE) returns (uint256) {
         return _accrueInterest();
     }
 
@@ -871,15 +871,15 @@ contract IonPool is IonPausableUpgradeable, RewardModule {
     /**
      * @return The total amount of collateral in the pool.
      */
-    function ilkCount() public view returns (uint256) {
+    function ilkCount() external view returns (uint256) {
         IonPoolStorage storage $ = _getIonPoolStorage();
         return $.ilks.length;
     }
 
     /**
      * @return The index of the collateral with `ilkAddress`.
      */
-    function getIlkIndex(address ilkAddress) public view returns (uint8) {
+    function getIlkIndex(address ilkAddress) external view returns (uint8) {
         IonPoolStorage storage $ = _getIonPoolStorage();
         bytes32 addressInBytes32 = bytes32(uint256(uint160(ilkAddress)));
 
@@ -891,27 +891,19 @@ contract IonPool is IonPausableUpgradeable, RewardModule {
     /**
      * @return The address of the collateral at index `ilkIndex`.
      */
-    function getIlkAddress(uint256 ilkIndex) public view returns (address) {
+    function getIlkAddress(uint256 ilkIndex) external view returns (address) {
         IonPoolStorage storage $ = _getIonPoolStorage();
         return $.ilkAddresses.at(ilkIndex);
     }
 
     /**
      * @return Whether or not an address is a supported collateral.
      */
-    function addressContains(address ilk) public view returns (bool) {
+    function addressContains(address ilk) external view returns (bool) {
         IonPoolStorage storage $ = _getIonPoolStorage();
         return $.ilkAddresses.contains(ilk);
     }
 
-    /**
-     * @return The total amount of addresses.
-     */
-    function addressesLength() public view returns (uint256) {
-        IonPoolStorage storage $ = _getIonPoolStorage();
-        return $.ilkAddresses.length();
-    }
-
     /**
      * @return The total amount of normalized debt for collateral with index
      * `ilkIndex`.
@@ -1064,7 +1056,7 @@ contract IonPool is IonPausableUpgradeable, RewardModule {
     /**
      * @dev Gets the current borrow rate for borrowing against a given collateral.
      */
-    function getCurrentBorrowRate(uint8 ilkIndex) public view returns (uint256 borrowRate, uint256 reserveFactor) {
+    function getCurrentBorrowRate(uint8 ilkIndex) external view returns (uint256 borrowRate, uint256 reserveFactor) {
         IonPoolStorage storage $ = _getIonPoolStorage();
 
         uint256 totalEthSupply = totalSupply();

src/Liquidation.sol

@@ -45,7 +45,7 @@ contract Liquidation {
     IERC20 public immutable UNDERLYING;
 
     // --- Events ---
-    event Liquidate(address indexed kpr, uint8 indexed ilkIndex, uint256 repay, uint256 gemOut);
+    event Liquidate(address indexed initiator, address indexed kpr, uint8 indexed ilkIndex, uint256 repay, uint256 gemOut);
 
     constructor(
         address _ionPool,
@@ -93,6 +93,7 @@ contract Liquidation {
         uint256 maxDiscount; 
         address reserveOracle; 
     }
+
     /**
      * @notice Returns the exchange rate and liquidation threshold for the given ilkIndex.
      */
@@ -101,7 +102,6 @@ contract Liquidation {
         view
         returns (Configs memory configs)
     {
-        address reserveOracle;
         if (ilkIndex == 0) {
             configs.reserveOracle = RESERVE_ORACLE_0;
             configs.liquidationThreshold = LIQUIDATION_THRESHOLD_0;
@@ -165,7 +165,7 @@ contract Liquidation {
         Configs memory configs = _getConfigs(ilkIndex);
 
         // exchangeRate is reported in uint72 in [wad], but should be converted to uint256 [ray]
-        uint256 exchangeRate = uint256(ReserveOracle(configs.reserveOracle).currentExchangeRate()).scaleUpToRay(18);
+        uint256 exchangeRate = ReserveOracle(configs.reserveOracle).currentExchangeRate().scaleUpToRay(18);
         (uint256 collateral, uint256 normalizedDebt) = POOL.vault(ilkIndex, vault);
         uint256 rate = POOL.rate(ilkIndex);
 
@@ -216,7 +216,7 @@ contract Liquidation {
             POOL.confiscateVault(
                 ilkIndex, vault, PROTOCOL, PROTOCOL, -int256(liquidateArgs.gemOut), -int256(liquidateArgs.dart)
             );
-            emit Liquidate(kpr, ilkIndex, liquidateArgs.dart, liquidateArgs.gemOut);
+            emit Liquidate(msg.sender, kpr, ilkIndex, liquidateArgs.dart, liquidateArgs.gemOut);
             return; // early return
         } else if (normalizedDebt * rate - liquidateArgs.repay < POOL.dust(ilkIndex)) {
             // [rad] - [rad] < [rad]
@@ -251,6 +251,6 @@ contract Liquidation {
         // pay off the unbacked debt
         POOL.repayBadDebt(address(this), liquidateArgs.repay);
 
-        emit Liquidate(kpr, ilkIndex, liquidateArgs.dart, liquidateArgs.gemOut);
+        emit Liquidate(msg.sender, kpr, ilkIndex, liquidateArgs.dart, liquidateArgs.gemOut);
     }
 }

src/Whitelist.sol

@@ -6,16 +6,14 @@ import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
 import { MerkleProof } from "@openzeppelin/contracts/utils/cryptography/MerkleProof.sol";
 
 contract Whitelist is Ownable2Step {
-    mapping(address => bool) public protocolWhitelist; // peripheral addresses that can bypass the merkle proof check
+    mapping(address protocolControlledAddress => bool) public protocolWhitelist; // peripheral addresses that can bypass the merkle proof check
 
-    mapping(uint8 => bytes32) public borrowersRoot; // root of the merkle tree of borrowers for each ilk
+    mapping(uint8 ilkIndex => bytes32) public borrowersRoot; // root of the merkle tree of borrowers for each ilk
 
     bytes32 public lendersRoot; // root of the merkle tree of lenders for each ilk
 
     // --- Errors ---
 
-    error InvalidConstructorArguments();
-    error InvalidWhitelistMerkleProof();
     error NotWhitelistedBorrower(uint8 ilkIndex, address addr);
     error NotWhitelistedLender(address addr);
 

src/admin/IonPausableUpgradeable.sol

@@ -12,7 +12,7 @@ import { ContextUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/Co
  * actions that put the protocol into a further unsafe state (e.g. borrows,
  * withdraws of base), one for pausing actions that put the protocol into a
  * safer state (e.g. repays, deposits of base), and one for pausing the accrual
- * of interest rates. Depnding on the situation, it may be desirable to pause
+ * of interest rates. Depending on the situation, it may be desirable to pause
  * one, two, or all of these, hence the reasoning for this design.
  */
 abstract contract IonPausableUpgradeable is ContextUpgradeable {
@@ -21,6 +21,7 @@ abstract contract IonPausableUpgradeable is ContextUpgradeable {
         SAFE
     }
 
+    /// @custom:storage-location erc7201:ion.storage.IonPausable
     struct IonPausableStorage {
         // Initialized to unpaused implicitly
         bool[2] _pausedStates;

src/flash/handlers/EthXHandler.sol

@@ -21,17 +21,18 @@ contract EthXHandler is UniswapFlashloanBalancerSwapHandler, BalancerFlashloanDi
     using StaderLibrary for IStaderStakePoolsManager;
 
     // Stader deposit contract is separate from the ETHx lst contract
-    IStaderStakePoolsManager immutable STADER_DEPOSIT;
+    IStaderStakePoolsManager immutable public STADER_DEPOSIT;
 
     constructor(
         uint8 _ilkIndex,
         IonPool _ionPool,
         GemJoin _gemJoin,
         IStaderStakePoolsManager _staderDeposit,
         Whitelist _whitelist,
-        IUniswapV3Pool _wstEthUniswapPool
+        IUniswapV3Pool _wstEthUniswapPool,
+        bytes32 _balancerPoolId
     )
-        UniswapFlashloanBalancerSwapHandler(_wstEthUniswapPool)
+        UniswapFlashloanBalancerSwapHandler(_wstEthUniswapPool, _balancerPoolId)
         IonHandlerBase(_ilkIndex, _ionPool, _gemJoin, _whitelist)
     {
         STADER_DEPOSIT = _staderDeposit;

src/flash/handlers/SwEthHandler.sol

@@ -23,12 +23,10 @@ contract SwEthHandler is UniswapFlashswapHandler, BalancerFlashloanDirectMintHan
         IonPool _ionPool,
         GemJoin _gemJoin,
         Whitelist _whitelist,
-        IUniswapV3Factory _factory,
-        IUniswapV3Pool _swEthPool,
-        uint24 _poolFee
+        IUniswapV3Pool _swEthPool
     )
         IonHandlerBase(_ilkIndex, _ionPool, _gemJoin, _whitelist)
-        UniswapFlashswapHandler(_factory, _swEthPool, _poolFee, true)
+        UniswapFlashswapHandler(_swEthPool, true)
     { }
 
     function _depositWethForLst(uint256 wethAmount) internal override returns (uint256) {

src/flash/handlers/WstEthHandler.sol

@@ -21,13 +21,11 @@ contract WstEthHandler is UniswapFlashswapHandler, BalancerFlashloanDirectMintHa
         IonPool _ionPool,
         GemJoin _gemJoin,
         Whitelist _whitelist,
-        IUniswapV3Factory _factory,
-        IUniswapV3Pool _wstEthUniswapPool,
-        uint24 _poolFee
+        IUniswapV3Pool _wstEthUniswapPool
     )
         IonHandlerBase(_ilkIndex, _ionPool, _gemJoin, _whitelist)
         // token0 is wstEth
-        UniswapFlashswapHandler(_factory, _wstEthUniswapPool, _poolFee, false)
+        UniswapFlashswapHandler(_wstEthUniswapPool, false)
     { }
 
     function _depositWethForLst(uint256 amountWeth) internal override returns (uint256) {

src/flash/handlers/base/BalancerFlashloanDirectMintHandler.sol

@@ -59,18 +59,18 @@ abstract contract BalancerFlashloanDirectMintHandler is IonHandlerBase, IFlashLo
 
         uint256 amountToLeverage = resultingAdditionalCollateral - initialDeposit; // in collateral terms
 
+        if (amountToLeverage == 0) {
+            // AmountToBorrow.IS_MAX because we don't want to create any new debt here
+            _depositAndBorrow(msg.sender, address(this), resultingAdditionalCollateral, 0, AmountToBorrow.IS_MAX);
+            return;
+        }
+
         IERC20Balancer[] memory addresses = new IERC20Balancer[](1);
         addresses[0] = IERC20Balancer(address(LST_TOKEN));
 
         uint256[] memory amounts = new uint256[](1);
         amounts[0] = amountToLeverage;
 
-        if (amounts[0] == 0) {
-            // AmountToBorrow.IS_MAX because we don't want to create any new debt here
-            _depositAndBorrow(msg.sender, address(this), resultingAdditionalCollateral, 0, AmountToBorrow.IS_MAX);
-            return;
-        }
-
         uint256 wethRequiredForRepayment = _getEthAmountInForLstAmountOut(amountToLeverage);
         if (wethRequiredForRepayment > maxResultingDebt) {
             revert FlashloanRepaymentTooExpensive(wethRequiredForRepayment, maxResultingDebt);