diff --git a/src/Constants.sol b/src/Constants.sol index 03cca83..c4f86f2 100644 --- a/src/Constants.sol +++ b/src/Constants.sol @@ -3,3 +3,5 @@ pragma solidity =0.8.18; uint256 constant RESOLUTION = 1e18; uint256 constant ONE_YEAR = 365 days; +uint256 constant MINIMUM_FEE_UNLOCK_TIME = 30 seconds; +uint256 constant MAXIMUM_FEE_UNLOCK_TIME = 7 days; diff --git a/src/Vault.sol b/src/Vault.sol index e7274a7..f7aa905 100644 --- a/src/Vault.sol +++ b/src/Vault.sol @@ -8,6 +8,7 @@ import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.s import { ERC20, ERC20Permit } from "@openzeppelin/contracts/token/ERC20/extensions/draft-ERC20Permit.sol"; import { ERC4626, IERC4626 } from "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; import { IVault } from "./interfaces/IVault.sol"; +import { MINIMUM_FEE_UNLOCK_TIME, MAXIMUM_FEE_UNLOCK_TIME } from "./Constants.sol"; // Since this vault inherits from ERC4626, it has the known vulnerability of the "donation attack" // In the Ithil framework this is fixed by deploying the Vault already with 1 token deposited @@ -58,9 +59,8 @@ contract Vault is IVault, ERC4626, ERC20Permit { } function setFeeUnlockTime(uint256 _feeUnlockTime) external override onlyOwner { - // Minimum 30 seconds, maximum 7 days - // This also avoids division by zero in _calculateLockedProfits() - if (_feeUnlockTime < 30 seconds || _feeUnlockTime > 7 days) revert FeeUnlockTimeOutOfRange(); + if (_feeUnlockTime < MINIMUM_FEE_UNLOCK_TIME || _feeUnlockTime > MAXIMUM_FEE_UNLOCK_TIME) + revert FeeUnlockTimeOutOfRange(); currentProfits = _calculateLockedProfits(); currentLosses = _calculateLockedLosses(); latestRepay = block.timestamp; @@ -230,7 +230,7 @@ contract Vault is IVault, ERC4626, ERC20Permit { currentLosses = _calculateLockedLosses() + (assets - loan); latestRepay = block.timestamp; - IERC20(asset()).safeTransfer(receiver, assets); + if (assets > 0) IERC20(asset()).safeTransfer(receiver, assets); emit Borrowed(receiver, assets); @@ -265,7 +265,7 @@ contract Vault is IVault, ERC4626, ERC20Permit { // the vault is not responsible for any payoff // slither-disable-next-line arbitrary-send-erc20 // super.totalAssets() += assets and never overflows by definition - IERC20(asset()).safeTransferFrom(repayer, address(this), assets); + if (assets > 0) IERC20(asset()).safeTransferFrom(repayer, address(this), assets); emit Repaid(repayer, assets, debt); } diff --git a/src/irmodels/AuctionRateModel.sol b/src/irmodels/AuctionRateModel.sol index f216051..c46eca8 100644 --- a/src/irmodels/AuctionRateModel.sol +++ b/src/irmodels/AuctionRateModel.sol @@ -9,6 +9,7 @@ import { BaseRiskModel } from "../services/BaseRiskModel.sol"; /// Rate model in which baseIR is based on a Dutch auction /// 1e18 corresponds to 1, i.e. an interest rate of 100% abstract contract AuctionRateModel is Ownable, BaseRiskModel { + uint256 private constant MAX_RATE = 1e18; /** * @dev gas saving trick * latest is a timestamp and base < 1e18, they all fit in uint256 @@ -25,7 +26,8 @@ abstract contract AuctionRateModel is Ownable, BaseRiskModel { error ZeroMarginLoan(); function setRiskParams(address token, uint256 riskSpread, uint256 baseRate, uint256 halfTime) external onlyOwner { - if (token == address(0) || baseRate > 1e18 || riskSpread > 1e18 || halfTime == 0) revert InvalidInitParams(); + if (token == address(0) || baseRate > MAX_RATE || riskSpread > MAX_RATE || halfTime == 0) + revert InvalidInitParams(); riskSpreads[token] = riskSpread; latestAndBase[token] = (block.timestamp << 128) + baseRate; halvingTime[token] = halfTime; @@ -74,7 +76,7 @@ abstract contract AuctionRateModel is Ownable, BaseRiskModel { freeLiquidity ); // Reset new base and latest borrow, force IR stays below resolution - if (newBase + spread > 1e18) revert InterestRateOverflow(); + if (newBase + spread > MAX_RATE) revert InterestRateOverflow(); latestAndBase[loan.token] = (block.timestamp << 128) + newBase; return (newBase, spread); diff --git a/src/services/credit/CallOption.sol b/src/services/credit/CallOption.sol index 4ad0e71..4177175 100644 --- a/src/services/credit/CallOption.sol +++ b/src/services/credit/CallOption.sol @@ -8,6 +8,7 @@ import { Service } from "../Service.sol"; import { IService } from "../../interfaces/IService.sol"; import { IVault } from "../../interfaces/IVault.sol"; import { Service } from "../Service.sol"; +import { RESOLUTION } from "../../Constants.sol"; /// @title Call option contract /// @author Ithil @@ -173,14 +174,15 @@ contract CallOption is CreditService { // 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(); + if (calledPortion > RESOLUTION || (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 - uint256 toCall = (agreement.collaterals[1].amount * calledPortion) / 1e18; + uint256 toCall = (agreement.collaterals[1].amount * calledPortion) / RESOLUTION; // The amount of ithil not called can be added back to the total allocation totalAllocation += (agreement.collaterals[1].amount - toCall); uint256 toTransfer = dueAmount(agreement, data); @@ -207,7 +209,7 @@ contract CallOption is CreditService { toBorrow = toTransfer - redeemed > freeLiquidity ? freeLiquidity : toTransfer - redeemed; } // We will always have ithil.balanceOf(address(this)) >= toCall, so the following succeeds - ithil.safeTransfer(ownerAddress, toCall); + if (toCall > 0) ithil.safeTransfer(ownerAddress, toCall); // repay the user's losses if (toBorrow > 0 && freeLiquidity > 0) manager.borrow(agreement.loans[0].token, toBorrow, 0, ownerAddress); } @@ -228,7 +230,7 @@ contract CallOption is CreditService { (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; + return (agreement.loans[0].amount * (RESOLUTION - calledPortion)) / RESOLUTION; } function allocateIthil(uint256 amount) external { diff --git a/src/services/debit/GmxService.sol b/src/services/debit/GmxService.sol index d9a91f9..b2b5fde 100644 --- a/src/services/debit/GmxService.sol +++ b/src/services/debit/GmxService.sol @@ -19,6 +19,8 @@ import { Service } from "../Service.sol"; contract GmxService is AuctionRateModel, DebitService { using SafeERC20 for IERC20; + uint256 private constant BASIS_POINTS_DIVISOR = 10000; + IRewardRouter public immutable router; IRewardRouterV2 public immutable routerV2; IERC20 public immutable glp; @@ -117,7 +119,7 @@ contract GmxService is AuctionRateModel, DebitService { totalRewards = toTransfer < newRewards ? newRewards - toTransfer : 0; totalCollateral -= agreement.collaterals[0].amount; // Transfer weth: since toTransfer <= totalWithdraw - weth.safeTransfer(msg.sender, toTransfer); + if (toTransfer > 0) weth.safeTransfer(msg.sender, toTransfer); } function wethReward(uint256 tokenID) public view returns (uint256) { @@ -151,7 +153,7 @@ contract GmxService is AuctionRateModel, DebitService { usdgVault.taxBasisPoints(), false ); - results[0] = (usdgDelta * (10000 - feeBasisPoints)) / 10000; + results[0] = (usdgDelta * (BASIS_POINTS_DIVISOR - feeBasisPoints)) / BASIS_POINTS_DIVISOR; return results; }