Fix Review #1
Fix Review #1
Conversation
| uint160 public maxSpotOffsetPlus1SqrtBps = 10072; | ||
| uint160 public maxSpotOffsetMinus1SqrtBps = 9927; |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#7.
Fixes issues: #175
|
|
||
| nuAssetManager public nuAManager; | ||
|
|
||
| event IntervalShort(uint32 _intervalShort); | ||
| event IntervalLong(uint32 _intervalLong); | ||
| event MaxSpotOffsetBps(uint _maxSpotOffsetBps); | ||
| event MaxSpotOffset(uint _maxSpotOffset); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#7.
Fixes issues: #175
| @@ -69,20 +69,18 @@ contract NumaOracle is Ownable2Step, INumaOracle { | |||
|
|
|||
| /** | |||
| * | |||
| * @param _maxSpotOffsetBps offset percentage variable (cf doc) | |||
| * @param _maxSpotOffset offset percentage variable (cf doc) | |||
There was a problem hiding this comment.
Change related to NumaMoney/Numa#7.
Fixes issues: #175
| function setMaxSpotOffset(uint _maxSpotOffset) external onlyOwner { | ||
| require(_maxSpotOffset < 1 ether, "percentage must be less than 100"); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#7.
Fixes issues: #175
| maxSpotOffsetPlus1SqrtBps = | ||
| 100 * | ||
| uint160(Math.sqrt(10000 + _maxSpotOffsetBps)); | ||
| uint160(Math.sqrt(1 ether + _maxSpotOffset))/1e5; |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#7.
Fixes issues: #175
| maxSpotOffsetMinus1SqrtBps = | ||
| 100 * | ||
| uint160(Math.sqrt(10000 - _maxSpotOffsetBps)); | ||
| uint160(Math.sqrt(1 ether - _maxSpotOffset))/1e5; |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#7.
Fixes issues: #175
|
|
||
| emit MaxSpotOffsetBps(_maxSpotOffsetBps); | ||
| emit MaxSpotOffset(_maxSpotOffset); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#7.
Fixes issues: #175
| // sherlock issue 41 | ||
| // CF warning can be bypassed |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#8.
Fixes issues: #41
| modifier notInWarningCF() { | ||
| _; |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#8.
Fixes issues: #41
| @@ -449,7 +451,7 @@ contract NumaPrinter is Pausable, Ownable2Step { | |||
|
|
|||
| (uint scaleSynthBurn, , , ) = vaultManager.getSynthScaling(); | |||
| // apply scale | |||
| costWithoutFee = (costWithoutFee * scaleSynthBurn) / BASE_1000; | |||
| costWithoutFee = (costWithoutFee * scaleSynthBurn) / BASE_SCALE; | |||
There was a problem hiding this comment.
Change related to NumaMoney/Numa#6.
Fixes issues: #161
| @@ -493,7 +495,7 @@ contract NumaPrinter is Pausable, Ownable2Step { | |||
| uint256 nuAssetIn = oracle.ethToNuAssetRoundUp(_nuAsset, ethAmount); | |||
| (uint scaleSynthBurn, , , ) = vaultManager.getSynthScaling(); | |||
| // apply scale | |||
| nuAssetIn = (nuAssetIn * BASE_1000) / scaleSynthBurn; | |||
| nuAssetIn = (nuAssetIn * BASE_SCALE) / scaleSynthBurn; | |||
There was a problem hiding this comment.
Change related to NumaMoney/Numa#6.
Fixes issues: #161
| import "@openzeppelin/contracts_5.0.2/utils/structs/EnumerableSet.sol"; | ||
| import "../utils/constants.sol"; | ||
|
|
||
|
|
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| @@ -172,14 +173,47 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault { | |||
| } | |||
|
|
|||
| /** | |||
| * @dev minimum reth borrow balance needed to allow partial liquidations | |||
| * @dev minimum lst borrow balance needed to allow partial liquidations | |||
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| /** | ||
| * @dev minimum borrow balance needed to allow partial liquidations in numa or in lst | ||
| */ | ||
| function getMinBorrowAmountAllowPartialLiquidation(address _cBorrowToken) external view returns (uint) { | ||
| if (_cBorrowToken == address(cNuma) ) | ||
| { | ||
| // numa borrower | ||
| // min amount in numa | ||
| ( | ||
| , | ||
| , | ||
| uint criticalScaleForNumaPriceAndSellFee, | ||
| ) = vaultManager.getSynthScaling(); | ||
|
|
||
| uint minBorrowAmountAllowPartialLiquidationNuma = vaultManager | ||
| .tokenToNuma( | ||
| minBorrowAmountAllowPartialLiquidation, | ||
| last_lsttokenvalueWei, | ||
| decimals, | ||
| criticalScaleForNumaPriceAndSellFee | ||
| ); | ||
| return minBorrowAmountAllowPartialLiquidationNuma; | ||
|
|
||
| } | ||
| else | ||
| { | ||
| return minBorrowAmountAllowPartialLiquidation; | ||
|
|
||
| } | ||
|
|
||
|
|
||
| } | ||
|
|
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| @@ -855,11 +889,12 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault { | |||
|
|
|||
| function startLiquidation() | |||
| internal | |||
| returns (uint criticalScaleForNumaPriceAndSellFee) | |||
| returns (uint criticalScaleForNumaPriceAndSellFee, uint sell_fee) | |||
There was a problem hiding this comment.
Change related to NumaMoney/Numa#13.
| { | ||
| ( | ||
| , | ||
| criticalScaleForNumaPriceAndSellFee, | ||
| sell_fee |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#13.
| @@ -975,7 +1010,7 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault { | |||
| "invalid param" | |||
| ); | |||
|
|
|||
| uint criticalScaleForNumaPriceAndSellFee = startLiquidation(); | |||
| (uint criticalScaleForNumaPriceAndSellFee,) = startLiquidation(); | |||
There was a problem hiding this comment.
Change related to NumaMoney/Numa#13.
| ); | ||
| uint minAmount = minBorrowAmountAllowPartialLiquidationNuma; | ||
| if (borrowAmount < minAmount) minAmount = borrowAmount; | ||
| require(numaAmount >= minAmount, "min liquidation"); | ||
| } |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| @@ -1015,7 +1037,7 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault { | |||
|
|
|||
| // liquidate | |||
| numa.approve(address(cNuma), numaAmount); | |||
| cNuma.liquidateBorrow( | |||
| (,uint badDebt) = cNuma.liquidateBorrow( | |||
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // sherlock 86 42 | ||
| // for numa borrow, numa <-> lst conversions should use buyPrice | ||
| maxNumaProfitForLiquidations = (maxNumaProfitForLiquidations * vaultManager.getBuyFee()) / 1 ether; |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#13.
| // sherlock 101 153 | ||
| // cap only if there is no bad debt, because if we are in bad debt it means this is a partial liquidation which | ||
| // should not be capped | ||
| if ((badDebt == 0) && (numaLiquidatorProfit > maxNumaProfitForLiquidations)) |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // sherlock 86 42 | ||
| // for numa borrow, numa <-> lst conversions should use buyPrice | ||
| lstProvidedEstimate = (lstProvidedEstimate * 1 ether) / vaultManager.getBuyFee(); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#13.
|
|
||
| // cap profit | ||
| // sherlock 101 153 | ||
| // cap only if there is no bad debt, because if we are in bad debt it means this is a partial liquidation which | ||
| // should not be capped |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| uint vaultProfit; | ||
| if (lstLiquidatorProfit > maxLstProfitForLiquidations) { | ||
| if ((badDebt == 0) && (lstLiquidatorProfit > maxLstProfitForLiquidations)) { |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
|
|
||
| uint criticalScaleForNumaPriceAndSellFee = startLiquidation(); | ||
| (uint criticalScaleForNumaPriceAndSellFee,uint sellfee) = startLiquidation(); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#13.
| @@ -1151,7 +1184,7 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault { | |||
|
|
|||
| // liquidate | |||
| IERC20(lstToken).approve(address(cLstToken), lstAmount); | |||
| cLstToken.liquidateBorrow( | |||
| (,uint badDebt) = cLstToken.liquidateBorrow( | |||
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // sherlock 101 153 | ||
| // cap only if there is no bad debt, because if we are in bad debt it means this is a partial liquidation which | ||
| // should not be capped | ||
| if ((badDebt == 0) && (lstLiquidatorProfit > maxLstProfitForLiquidations)) |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // sherlock 86 42 | ||
| // for lst borrow, numa <-> lst conversions should use sellPrice | ||
| numaProvidedEstimate = (numaProvidedEstimate * 1 ether) / sellfee; | ||
|
|
There was a problem hiding this comment.
Change related to NumaMoney/Numa#13.
| // sherlock 86 42 | ||
| // for lst borrow, numa <-> lst conversions should use sellPrice | ||
| maxNumaProfitForLiquidations = (maxNumaProfitForLiquidations * 1 ether) / sellfee; | ||
|
|
There was a problem hiding this comment.
Change related to NumaMoney/Numa#13.
| @@ -129,5 +131,6 @@ contract TokenErrorReporter { | |||
| error ReduceReservesCashValidation(); | |||
|
|
|||
| error SetInterestRateModelOwnerCheck(); | |||
| error SetBorrowRateMaxMantissaOwnerCheck(); | |||
There was a problem hiding this comment.
Change related to NumaMoney/Numa#12.
Fixes issues: #192
| uint ltvMinBadDebtLiquidations = 0.98 ether; | ||
| uint ltvMinPartialLiquidations = 1.1 ether; | ||
|
|
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| * @param borrower The address of the borrower | ||
| * @param repayAmount The amount of underlying being repaid | ||
| * @return (error,baddebt,restOfDebt) |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| ) external view override returns (uint) { | ||
| // Shh - currently unused | ||
| liquidator; | ||
| ) external view override returns (uint,uint,uint) { |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| require((cTokenBorrowed) != (cTokenCollateral), "not isolate"); | ||
| if ( | ||
| !markets[cTokenBorrowed].isListed || | ||
| !markets[cTokenCollateral].isListed | ||
| ) { | ||
| return uint(Error.MARKET_NOT_LISTED); | ||
| return (uint(Error.MARKET_NOT_LISTED),0,0); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| , | ||
| uint shortfall, | ||
| uint badDebt, | ||
| uint ltv |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| CNumaToken(cTokenBorrowed) | ||
| ); | ||
| if (err != Error.NO_ERROR) { | ||
| return (uint(err),0,0); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
|
|
||
| // sherlock 101 153 | ||
| // min amount allowed for liquidations from vault | ||
| uint minAmount = 0; | ||
| bool badDebtLiquidationAllowed = false; | ||
|
|
||
| // Min amount from vault. | ||
| // we should have a vault, if not, it will revert which is ok | ||
| minAmount = CNumaToken(cTokenBorrowed).vault().getMinBorrowAmountAllowPartialLiquidation(cTokenBorrowed); | ||
|
|
||
| // if ltv > ltvMinPartialLiquidations, partial liquidations are enabled | ||
| if (ltv > ltvMinPartialLiquidations) | ||
| { | ||
| minAmount = 0; | ||
| // if ltv > ltvMinPartialLiquidations, it means we are in bad debt | ||
| // so we need to allow bad debt liquidation | ||
| badDebtLiquidationAllowed = true; | ||
| } | ||
|
|
||
| if (borrowBalance < minAmount) minAmount = borrowBalance; | ||
|
|
||
| require(repayAmount >= minAmount, "min liquidation"); | ||
|
|
||
|
|
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| if (badDebt > 0 && (!badDebtLiquidationAllowed)) { | ||
| return (uint(Error.BAD_DEBT),badDebt,0); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| if (shortfall == 0) | ||
| { | ||
| return (uint(Error.INSUFFICIENT_SHORTFALL),badDebt,0); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| @@ -607,14 +640,16 @@ contract NumaComptroller is | |||
| borrowBalance | |||
| ); | |||
| if (repayAmount > maxClose) { | |||
| return uint(Error.TOO_MUCH_REPAY); | |||
| return (uint(Error.TOO_MUCH_REPAY),badDebt,0); | |||
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| if (badDebt > 0 && (!badDebtLiquidationAllowed)) { | ||
| return (uint(Error.BAD_DEBT),badDebt,0); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // sherlock 101 153 returning badDebt and restOfDebt | ||
| // because we want to know if liquidation is partial and not in baddebt to allow taking all collateral | ||
| return (uint(Error.NO_ERROR),badDebt,borrowBalance - repayAmount); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| , | ||
| uint ltv |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| /* allow accounts to be liquidated if the market is deprecated */ | ||
| if (isDeprecated(CToken(cTokenBorrowed))) { | ||
| require( | ||
| borrowBalance >= repayAmount, | ||
| "Can not repay more than the total borrow" | ||
| ); | ||
| // sherlock issue 67. Even if deprecated some bad debt is needed | ||
| if (ltv < ltvMinBadDebtLiquidations) { |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // bad debt liquidation allowed only above ltvMinBadDebtLiquidations | ||
| if (ltv < ltvMinBadDebtLiquidations) { |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // function getAccountLTVIsolate( | ||
| // address account, | ||
| // CNumaToken collateral, | ||
| // CNumaToken borrow | ||
| // ) public view returns (uint, uint) { | ||
| // (Error err, uint ltv) = getAccountLTVIsolateInternal( | ||
| // account, | ||
| // collateral, | ||
| // borrow | ||
| // ); | ||
| // return (uint(err), ltv); | ||
| // } |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| if (vars.sumCollateralNoCollateralFactor > 0) | ||
| { | ||
| ltv = (vars.sumBorrowPlusEffects * 1 ether) / vars.sumCollateralNoCollateralFactor; | ||
| } | ||
| else if (vars.sumBorrowPlusEffects > 0) | ||
| { | ||
| // no collateral but some borrow, ltv is infinite | ||
| ltv = type(uint).max; |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // no collateral but some borrow, ltv is infinite | ||
| ltv = type(uint).max; | ||
|
|
||
| } |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // function getAccountLTVIsolateInternal( | ||
| // address account, | ||
| // CNumaToken collateral, | ||
| // CNumaToken borrow | ||
| // ) internal view returns (Error, uint) { | ||
| // AccountLiquidityLocalVars memory vars; // Holds all our calculation results | ||
| // uint oErr; |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // // Here we only consider only 2 tokens: collateral token and borrow token which should be different | ||
| // // collateral | ||
| // // Read the balances and exchange rate from the cToken | ||
| // ( | ||
| // oErr, | ||
| // vars.cTokenBalance, | ||
| // vars.borrowBalance, | ||
| // vars.exchangeRateMantissa | ||
| // ) = collateral.getAccountSnapshot(account); | ||
| // if (oErr != 0) { | ||
| // // semi-opaque error code, we assume NO_ERROR == 0 is invariant between upgrades | ||
| // return (Error.SNAPSHOT_ERROR, 0); | ||
| // } | ||
| // vars.collateralFactor = Exp({ | ||
| // mantissa: markets[address(collateral)].collateralFactorMantissa | ||
| // }); | ||
| // vars.exchangeRate = Exp({mantissa: vars.exchangeRateMantissa}); | ||
|
|
||
| // // Get the normalized price of the asset | ||
| // vars.oraclePriceMantissaCollateral = oracle | ||
| // .getUnderlyingPriceAsCollateral(collateral); | ||
| // if (vars.oraclePriceMantissaCollateral == 0) { | ||
| // return (Error.PRICE_ERROR, 0); | ||
| // } | ||
| // vars.oraclePriceCollateral = Exp({ | ||
| // mantissa: vars.oraclePriceMantissaCollateral | ||
| // }); | ||
|
|
||
| // // Pre-compute a conversion factor from tokens -> ether (normalized price value) | ||
| // vars.tokensToDenomCollateral = mul_( | ||
| // mul_(vars.collateralFactor, vars.exchangeRate), | ||
| // vars.oraclePriceCollateral | ||
| // ); | ||
| // vars.tokensToDenomCollateralNoCollateralFactor = mul_( | ||
| // vars.exchangeRate, | ||
| // vars.oraclePriceCollateral | ||
| // ); | ||
| // // sumCollateral += tokensToDenom * cTokenBalance |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // // NUMALENDING: use collateral price | ||
| // vars.sumCollateral = mul_ScalarTruncateAddUInt( | ||
| // vars.tokensToDenomCollateral, | ||
| // vars.cTokenBalance, | ||
| // vars.sumCollateral | ||
| // ); | ||
| // vars.sumCollateralNoCollateralFactor = mul_ScalarTruncateAddUInt( | ||
| // vars.tokensToDenomCollateralNoCollateralFactor, | ||
| // vars.cTokenBalance, | ||
| // vars.sumCollateralNoCollateralFactor | ||
| // ); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // // borrow | ||
| // // Read the balances and exchange rate from the cToken | ||
| // ( | ||
| // oErr, | ||
| // vars.cTokenBalance, | ||
| // vars.borrowBalance, | ||
| // vars.exchangeRateMantissa | ||
| // ) = borrow.getAccountSnapshot(account); | ||
| // if (oErr != 0) { | ||
| // // semi-opaque error code, we assume NO_ERROR == 0 is invariant between upgrades | ||
| // return (Error.SNAPSHOT_ERROR, 0); | ||
| // } | ||
| // // Get the normalized price of the asset | ||
| // vars.oraclePriceMantissaBorrowed = oracle.getUnderlyingPriceAsBorrowed( | ||
| // borrow | ||
| // ); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // if (vars.oraclePriceMantissaBorrowed == 0) { | ||
| // return (Error.PRICE_ERROR, 0); | ||
| // } | ||
| // //vars.oraclePriceCollateral = Exp({mantissa: vars.oraclePriceMantissaCollateral}); | ||
| // vars.oraclePriceBorrowed = Exp({ | ||
| // mantissa: vars.oraclePriceMantissaBorrowed | ||
| // }); | ||
|
|
||
| // // sumBorrowPlusEffects += oraclePrice * borrowBalance | ||
| // // NUMALENDING: use borrow price | ||
| // vars.sumBorrowPlusEffects = mul_ScalarTruncateAddUInt( | ||
| // vars.oraclePriceBorrowed, | ||
| // vars.borrowBalance, | ||
| // vars.sumBorrowPlusEffects | ||
| // ); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| // return ( | ||
| // Error.NO_ERROR, | ||
| // (vars.sumBorrowPlusEffects * 1 ether) / vars.sumCollateral | ||
| // ); | ||
| // } |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
| /** | ||
| * @notice Sets the ltv thresholds used when liquidating borrows | ||
| * @dev Admin function to set ltv thresholds | ||
| * @param _ltvMinBadDebt min ltv to allow bad debt liquidation | ||
| * @param _ltvMinPartialLiquidation min to allow partial liquidation | ||
| * @return uint 0=success, otherwise a failure | ||
| */ | ||
| function _setLtvThresholds( | ||
| uint _ltvMinBadDebt, | ||
| uint _ltvMinPartialLiquidation | ||
| ) external returns (uint) { | ||
| // Check caller is admin | ||
| require(msg.sender == admin, "only admin"); | ||
| ltvMinBadDebtLiquidations = _ltvMinBadDebt; | ||
| ltvMinPartialLiquidations = _ltvMinPartialLiquidation; | ||
|
|
||
| return uint(Error.NO_ERROR); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
|
|
||
| uint constant BASE_SCALE = 1 ether; |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#6.
Fixes issues: #161
| // 0.0005e16 for ethereum chain, should be 10 x less for arbitrum as blocktime is /10 | ||
| //uint borrowRateMaxMantissaARBI = 0.0005e16; | ||
| uint borrowRateMaxMantissaARBI = 0.00005e16; | ||
|
|
There was a problem hiding this comment.
Change related to NumaMoney/Numa#12.
Fixes issues: #192
| // arbitrum values | ||
| cReth._setBorrowRateMaxMantissa(borrowRateMaxMantissaARBI); | ||
| cNuma._setBorrowRateMaxMantissa(borrowRateMaxMantissaARBI); |
There was a problem hiding this comment.
Change related to NumaMoney/Numa#12.
Fixes issues: #192
| @@ -232,6 +238,7 @@ contract DeployLending is Script { | |||
| // 100% liquidation close factor | |||
| comptroller._setCloseFactor(closeFactor); | |||
| comptroller._setLiquidationIncentive(liquidationIncentive); | |||
| comptroller._setLtvThresholds(0.98 ether,1.1 ether); | |||
There was a problem hiding this comment.
Change related to NumaMoney/Numa#16.
Fix Review of
Repo:
NumaMoney/NumaCommit Hash:
e1d9b837cd0cf490d1c2a740591d87781a5a314c