solved merge conflicts and eliminated onlyWhitelisted modifier from s…

…ervices
Ithil-protocol · Dec 18, 2023 · 11d0922 · 11d0922
2 parents cdd3555 + 2f9668d
commit 11d0922
Show file tree

Hide file tree

Showing 17 changed files with 102 additions and 63 deletions.
diff --git a/src/Manager.sol b/src/Manager.sol
@@ -30,7 +30,7 @@ contract Manager is IManager, Ownable {
     }
 
     function create(address token) external onlyOwner returns (address) {
-        assert(vaults[token] == address(0));
+        if (vaults[token] != address(0)) revert VaultAlreadyExists();
 
         address vault = Create2.deploy(
             0,
@@ -61,21 +61,20 @@ contract Manager is IManager, Ownable {
         emit CapWasUpdated(service, token, percentageCap, absoluteCap);
     }
 
-    function setFeeUnlockTime(address vaultToken, uint256 feeUnlockTime) external override onlyOwner {
-        assert(vaults[vaultToken] != address(0));
-
+    function setFeeUnlockTime(
+        address vaultToken,
+        uint256 feeUnlockTime
+    ) external override vaultExists(vaultToken) onlyOwner {
         IVault(vaults[vaultToken]).setFeeUnlockTime(feeUnlockTime);
     }
 
-    function sweep(address vaultToken, address spuriousToken, address to) external onlyOwner {
-        assert(vaults[vaultToken] != address(0));
+    function sweep(address vaultToken, address spuriousToken, address to) external vaultExists(vaultToken) onlyOwner {
+        if (to == address(0)) revert InvalidParams();
 
         IVault(vaults[vaultToken]).sweep(to, spuriousToken);
     }
 
-    function toggleVaultLock(address vaultToken) external onlyOwner {
-        assert(vaults[vaultToken] != address(0));
-
+    function toggleVaultLock(address vaultToken) external vaultExists(vaultToken) onlyOwner {
         IVault(vaults[vaultToken]).toggleLock();
     }
 

diff --git a/src/Oracle.sol b/src/Oracle.sol
@@ -11,8 +11,12 @@ contract Oracle is IOracle, Ownable {
     event PriceFeedWasUpdated(address indexed token, address indexed feed);
 
     error TokenNotSupported();
+    error InvalidParams();
 
     function setPriceFeed(address token, address feed) external onlyOwner {
+        if (token == address(0)) revert InvalidParams();
+        if (feed == address(0)) revert InvalidParams();
+
         oracles[token] = feed;
 
         emit PriceFeedWasUpdated(token, feed);

diff --git a/src/interfaces/IManager.sol b/src/interfaces/IManager.sol
@@ -31,10 +31,12 @@ interface IManager {
     event CapWasUpdated(address indexed service, address indexed token, uint256 percentageCap, uint256 absoluteCap);
     event TokenWasRemovedFromService(address indexed service, address indexed token);
 
+    error VaultAlreadyExists();
     error VaultMissing();
     error RestrictedToWhitelisted();
     error RestrictedToOwner();
     error InvestmentCapExceeded(uint256 investedPortion);
     error AbsoluteCapExceeded(uint256 exposure);
     error MaxAmountExceeded();
+    error InvalidParams();
 }
diff --git a/src/interfaces/IService.sol b/src/interfaces/IService.sol
@@ -64,5 +64,5 @@ interface IService is IERC721Enumerable {
     error RestrictedToOwner();
     error RestrictedAccess();
     error InvalidStatus();
-    error InvalidArguments();
+    error InvalidParams();
 }
diff --git a/src/irmodels/AuctionRateModel.sol b/src/irmodels/AuctionRateModel.sol
@@ -25,7 +25,7 @@ abstract contract AuctionRateModel is Ownable, BaseRiskModel {
     mapping(address => uint256) public latestAndBase;
 
     function setRiskParams(address token, uint256 riskSpread, uint256 baseRate, uint256 halfTime) external onlyOwner {
-        if (baseRate > 1e18 || riskSpread > 1e18 || halfTime == 0) revert InvalidInitParams();
+        if (token == address(0) || baseRate > 1e18 || riskSpread > 1e18 || halfTime == 0) revert InvalidInitParams();
         riskSpreads[token] = riskSpread;
         latestAndBase[token] = (block.timestamp << 128) + baseRate;
         halvingTime[token] = halfTime;

diff --git a/src/services/Service.sol b/src/services/Service.sol
@@ -55,6 +55,8 @@ abstract contract Service is IService, ERC721Enumerable, Ownable {
     ///// Admin functions /////
 
     function setGuardian(address _guardian) external onlyOwner {
+        if (_guardian == address(0)) revert InvalidParams();
+
         guardian = _guardian;
 
         emit GuardianWasUpdated(guardian);
@@ -93,9 +95,9 @@ abstract contract Service is IService, ERC721Enumerable, Ownable {
 
         _saveAgreement(agreement);
 
-        _safeMint(msg.sender, id++);
+        _safeMint(msg.sender, id);
 
-        emit PositionOpened(id, msg.sender, agreement);
+        emit PositionOpened(id++, msg.sender, agreement);
     }
 
     /// @notice closes an existing service agreement

diff --git a/src/services/credit/CallOption.sol b/src/services/credit/CallOption.sol
@@ -47,6 +47,7 @@ contract CallOption is CreditService {
     error InvalidIthilToken();
     error InvalidUnderlyingToken();
     error InvalidCalledPortion();
+    error RedeemedTooLow();
     error SlippageExceeded();
     error StillVested();
 
@@ -69,7 +70,11 @@ contract CallOption is CreditService {
             13 * 30 * 86400
         )
     {
-        assert(_initialPrice > 0);
+        if (_initialPrice == 0) revert InvalidParams();
+        if (_manager == address(0)) revert InvalidParams();
+        if (_ithil == address(0)) revert InvalidParams();
+        if (_initialPrice == 0) revert InvalidParams();
+        if (_underlying == address(0)) revert InvalidParams();
 
         initialPrice = _initialPrice;
         underlying = IERC20(_underlying);
@@ -101,8 +106,8 @@ contract CallOption is CreditService {
 
     function _open(Agreement memory agreement, bytes memory data) internal override {
         // This is a credit service with one extra token (Ithil) therefore the collateral length is 2
-        if (agreement.loans.length != 1) revert InvalidArguments();
-        if (agreement.collaterals.length != 2) revert InvalidArguments();
+        if (agreement.loans.length != 1) revert InvalidParams();
+        if (agreement.collaterals.length != 2) revert InvalidParams();
 
         if (agreement.loans[0].token != address(underlying)) revert InvalidUnderlyingToken();
         if (agreement.collaterals[1].token != address(ithil)) revert InvalidIthilToken();
@@ -162,10 +167,13 @@ contract CallOption is CreditService {
         if (block.timestamp < agreement.createdAt + deadline - tenorDuration) revert LockPeriodStillActive();
         // The portion of the loan amount we want to call
         // If the position is liquidable, we enforce the option not to be exercised
-        uint256 calledPortion = abi.decode(data, (uint256));
+        // We also add a slippage check to protect signers from liquidity crunch attacks
+        (uint256 calledPortion, uint256 minRedeemed) = abi.decode(data, (uint256, uint256));
         address ownerAddress = ownerOf(tokenID);
         if (calledPortion > 1e18 || (msg.sender != ownerAddress && calledPortion > 0)) revert InvalidCalledPortion();
 
+        uint256 toBorrow;
+        uint256 freeLiquidity;
         // redeem mechanism
         IVault vault = IVault(manager.vaults(agreement.loans[0].token));
         // calculate the amount of shares to redeem to get dueAmount
@@ -174,28 +182,26 @@ contract CallOption is CreditService {
         totalAllocation += (agreement.collaterals[1].amount - toCall);
         uint256 toTransfer = dueAmount(agreement, data);
         uint256 toRedeem = vault.convertToShares(toTransfer);
-        uint256 transfered = vault.convertToAssets(
-            toRedeem < agreement.collaterals[0].amount ? toRedeem : agreement.collaterals[0].amount
-        );
-        uint256 toBorrow;
-        uint256 freeLiquidity;
+
         // If the called portion is not 100%, there are residual tokens which are transferred to the treasury
         if (toRedeem < agreement.collaterals[0].amount) {
             vault.safeTransfer(owner(), agreement.collaterals[0].amount - toRedeem);
         }
         // redeem the user's tokens and give the proceedings back to the user
-        vault.redeem(
+        uint256 redeemed = vault.redeem(
             toRedeem < agreement.collaterals[0].amount ? toRedeem : agreement.collaterals[0].amount,
             ownerAddress,
             address(this)
         );
-        if (toTransfer > transfered) {
+        if (redeemed < minRedeemed) revert RedeemedTooLow();
+
+        if (toTransfer > redeemed) {
             // Since this service is senior, we need to pay the user even if withdraw amount is too low
             // To do this, we take liquidity from the vault and register the loss
             // If we incur a loss and the freeLiquidity is not enough, we cannot make the exit fail
             // Otherwise we would have positions impossible to close: thus we withdraw what we can
             freeLiquidity = vault.freeLiquidity() - 1;
-            toBorrow = toTransfer - transfered > freeLiquidity ? freeLiquidity : toTransfer - transfered;
+            toBorrow = toTransfer - redeemed > freeLiquidity ? freeLiquidity : toTransfer - redeemed;
         }
         // We will always have ithil.balanceOf(address(this)) >= toCall, so the following succeeds
         ithil.safeTransfer(ownerAddress, toCall);
@@ -216,7 +222,7 @@ contract CallOption is CreditService {
 
     function dueAmount(Agreement memory agreement, bytes memory data) public view virtual override returns (uint256) {
         // The portion of the loan amount we want to call
-        uint256 calledPortion = abi.decode(data, (uint256));
+        (uint256 calledPortion, ) = abi.decode(data, (uint256, uint256));
 
         // The non-called portion is capital to give back to the user
         return (agreement.loans[0].amount * (1e18 - calledPortion)) / 1e18;

diff --git a/src/services/credit/FixedYieldService.sol b/src/services/credit/FixedYieldService.sol
@@ -25,12 +25,13 @@ contract FixedYieldService is CreditService {
         uint256 _yield,
         uint256 _deadline
     ) Service("Fixed Yield Service", "FIXED-YIELD-SERVICE", _manager, _deadline) {
+        if (_manager == address(0)) revert InvalidParams();
         yield = _yield;
     }
 
     function _open(IService.Agreement memory agreement, bytes memory /*data*/) internal virtual override {
-        if (agreement.loans.length != 1) revert InvalidArguments();
-        if (agreement.collaterals.length != 2) revert InvalidArguments();
+        if (agreement.loans.length != 1) revert InvalidParams();
+        if (agreement.collaterals.length != 2) revert InvalidParams();
 
         address vaultAddress = manager.vaults(agreement.loans[0].token);
         if (IERC20(agreement.loans[0].token).allowance(address(this), vaultAddress) < agreement.loans[0].amount) {

diff --git a/src/services/debit/AaveService.sol b/src/services/debit/AaveService.sol
@@ -27,12 +27,14 @@ contract AaveService is AuctionRateModel, DebitService {
         address _aave,
         uint256 _deadline
     ) Service("AaveService", "AAVE-SERVICE", _manager, _deadline) {
+        if (_manager == address(0)) revert InvalidParams();
+        if (_aave == address(0)) revert InvalidParams();
         aave = IPool(_aave);
     }
 
     function _open(Agreement memory agreement, bytes memory /*data*/) internal override {
-        if (agreement.loans.length != 1) revert InvalidArguments();
-        if (agreement.collaterals.length != 1) revert InvalidArguments();
+        if (agreement.loans.length != 1) revert InvalidParams();
+        if (agreement.collaterals.length != 1) revert InvalidParams();
 
         IAToken aToken = IAToken(agreement.collaterals[0].token);
         if (aToken.UNDERLYING_ASSET_ADDRESS() != agreement.loans[0].token) revert IncorrectObtainedToken();

diff --git a/src/services/debit/AngleService.sol b/src/services/debit/AngleService.sol
@@ -26,14 +26,17 @@ contract AngleService is AuctionRateModel, DebitService {
         address _steur,
         uint256 _deadline
     ) Service("AngleService", "ANGLE-SERVICE", _manager, _deadline) {
+        if (_manager == address(0)) revert InvalidParams();
+        if (_steur == address(0)) revert InvalidParams();
+
         stEur = IERC4626(_steur);
         agEur = IERC20(stEur.asset());
         agEur.approve(address(stEur), type(uint256).max);
     }
 
     function _open(Agreement memory agreement, bytes memory /*data*/) internal override {
-        if (agreement.loans.length != 1) revert InvalidArguments();
-        if (agreement.collaterals.length != 1) revert InvalidArguments();
+        if (agreement.loans.length != 1) revert InvalidParams();
+        if (agreement.collaterals.length != 1) revert InvalidParams();
         if (agreement.loans[0].token != address(agEur)) revert IncorrectProvidedToken();
         if (agreement.collaterals[0].token != address(stEur)) revert IncorrectObtainedToken();
 

diff --git a/src/services/debit/BalancerService.sol b/src/services/debit/BalancerService.sol
@@ -58,6 +58,12 @@ contract BalancerService is AuctionRateModel, DebitService {
         address _bal,
         uint256 _deadline
     ) Service("BalancerService", "BALANCER-SERVICE", _manager, _deadline) {
+        if (_manager == address(0)) revert InvalidParams();
+        if (_oracle == address(0)) revert InvalidParams();
+        if (_factory == address(0)) revert InvalidParams();
+        if (_balancerVault == address(0)) revert InvalidParams();
+        if (_bal == address(0)) revert InvalidParams();
+
         oracle = IOracle(_oracle);
         dex = IFactory(_factory);
         balancerVault = IBalancerVault(_balancerVault);

diff --git a/src/services/debit/FraxlendService.sol b/src/services/debit/FraxlendService.sol
@@ -26,14 +26,17 @@ contract FraxlendService is AuctionRateModel, DebitService {
         address _fraxLend,
         uint256 _deadline
     ) Service("FraxlendService", "FRAXLEND-SERVICE", _manager, _deadline) {
+        if (_manager == address(0)) revert InvalidParams();
+        if (_fraxLend == address(0)) revert InvalidParams();
+
         fraxLend = IERC4626(_fraxLend);
         frax = IERC20(fraxLend.asset());
         frax.approve(address(fraxLend), type(uint256).max);
     }
 
     function _open(Agreement memory agreement, bytes memory /*data*/) internal override {
-        if (agreement.loans.length != 1) revert InvalidArguments();
-        if (agreement.collaterals.length != 1) revert InvalidArguments();
+        if (agreement.loans.length != 1) revert InvalidParams();
+        if (agreement.collaterals.length != 1) revert InvalidParams();
         if (agreement.loans[0].token != address(frax)) revert IncorrectProvidedToken();
         if (agreement.collaterals[0].token != address(fraxLend)) revert IncorrectObtainedToken();
 

diff --git a/src/services/debit/GmxService.sol b/src/services/debit/GmxService.sol
@@ -45,6 +45,10 @@ contract GmxService is AuctionRateModel, DebitService {
         address _routerV2,
         uint256 _deadline
     ) Service("GmxService", "GMX-SERVICE", _manager, _deadline) {
+        if (_manager == address(0)) revert InvalidParams();
+        if (_router == address(0)) revert InvalidParams();
+        if (_routerV2 == address(0)) revert InvalidParams();
+
         router = IRewardRouter(_router);
         routerV2 = IRewardRouterV2(_routerV2);
         glp = IERC20(routerV2.glp());
@@ -56,8 +60,8 @@ contract GmxService is AuctionRateModel, DebitService {
     }
 
     function _open(Agreement memory agreement, bytes memory /*data*/) internal override {
-        if (agreement.loans.length != 1) revert InvalidArguments();
-        if (agreement.collaterals.length != 1) revert InvalidArguments();
+        if (agreement.loans.length != 1) revert InvalidParams();
+        if (agreement.collaterals.length != 1) revert InvalidParams();
 
         if (IERC20(agreement.loans[0].token).allowance(address(this), address(glpManager)) == 0) {
             IERC20(agreement.loans[0].token).approve(address(glpManager), type(uint256).max);
@@ -82,6 +86,10 @@ contract GmxService is AuctionRateModel, DebitService {
 
     function _close(uint256 tokenID, Agreement memory agreement, bytes memory data) internal override {
         uint256 minAmountOut = abi.decode(data, (uint256));
+        uint256 userVirtualDeposit = virtualDeposit[tokenID];
+        // delete virtual deposits
+        delete virtualDeposit[tokenID];
+        totalVirtualDeposits -= userVirtualDeposit;
 
         routerV2.unstakeAndRedeemGlp(
             agreement.loans[0].token,
@@ -100,16 +108,11 @@ contract GmxService is AuctionRateModel, DebitService {
             totalCollateral;
         // Subtracting the virtual deposit we get the weth part: this is the weth the user is entitled to
         // Due to integer arithmetic, we may get underflow if we do not make checks
-        uint256 toTransfer = totalWithdraw >= virtualDeposit[tokenID]
-            ? totalWithdraw - virtualDeposit[tokenID] <= finalBalance
-                ? totalWithdraw - virtualDeposit[tokenID]
-                : finalBalance
+        uint256 toTransfer = totalWithdraw >= userVirtualDeposit
+            ? totalWithdraw - userVirtualDeposit <= finalBalance ? totalWithdraw - userVirtualDeposit : finalBalance
             : 0;
-        // delete virtual deposits
-        totalVirtualDeposits -= virtualDeposit[tokenID];
-        delete virtualDeposit[tokenID];
-        // update totalRewards and totalCollateral
-        totalRewards = newRewards - toTransfer;
+        // update totalRewards and totalCollateral with an extra check to avoid integer arithmetic underflows
+        totalRewards = toTransfer < newRewards ? newRewards - toTransfer : 0;
         totalCollateral -= agreement.collaterals[0].amount;
         // Transfer weth: since toTransfer <= totalWithdraw
         weth.safeTransfer(msg.sender, toTransfer);

diff --git a/src/services/neutral/FeeCollectorService.sol b/src/services/neutral/FeeCollectorService.sol
@@ -55,8 +55,12 @@ contract FeeCollectorService is Service {
         address _oracle,
         address _dex
     ) Service("FeeCollector", "FEE-COLLECTOR", _manager, 13 * 30 * 86400) {
-        veToken = new VeIthil();
+        if (_manager == address(0)) revert InvalidParams();
+        if (_weth == address(0)) revert InvalidParams();
+        if (_oracle == address(0)) revert InvalidParams();
+        if (_dex == address(0)) revert InvalidParams();
 
+        veToken = new VeIthil();
         weth = IERC20(_weth);
         oracle = IOracle(_oracle);
         dex = IFactory(_dex);
@@ -91,7 +95,7 @@ contract FeeCollectorService is Service {
         if (agreement.loans[0].margin == 0) revert ZeroAmount();
         if (weights[agreement.loans[0].token] == 0) revert UnsupportedToken();
         // gas savings
-        uint256 totalAssets = totalAssets();
+        uint256 totalAssetsCache = totalAssets();
         // Apply reward based on lock
         uint256 monthsLocked = abi.decode(data, (uint256));
         if (monthsLocked > 11) revert MaxLockExceeded();
@@ -108,9 +112,9 @@ contract FeeCollectorService is Service {
             1e36;
 
         // we assign a virtual deposit of v * A / S, __afterwards__ we update the total deposits
-        virtualDeposit[id] = totalAssets == 0
+        virtualDeposit[id] = totalAssetsCache == 0
             ? agreement.collaterals[0].amount
-            : (agreement.collaterals[0].amount * totalAssets) / veToken.totalSupply();
+            : (agreement.collaterals[0].amount * totalAssetsCache) / veToken.totalSupply();
         totalVirtualDeposits += virtualDeposit[id];
 
         veToken.mint(msg.sender, agreement.collaterals[0].amount);
@@ -159,10 +163,10 @@ contract FeeCollectorService is Service {
     function withdrawable(uint256 tokenId) public view returns (uint256) {
         Agreement memory agreement = agreements[tokenId];
         // gas savings
-        uint256 totalAssets = totalAssets();
+        uint256 totalAssetsCache = totalAssets();
         uint256 totalSupply = veToken.totalSupply();
         // This is the total withdrawable, consisting of weth + virtual deposit
-        uint256 totalWithdraw = (totalAssets * agreement.collaterals[0].amount) / totalSupply;
+        uint256 totalWithdraw = (totalAssetsCache * agreement.collaterals[0].amount) / totalSupply;
         // Subtracting the virtual deposit we get the weth part: this is the weth the user is entitled to
         return totalWithdraw - virtualDeposit[tokenId];
     }