From 76d67ab779302b90ec0dee4e3a467723a7983d0a Mon Sep 17 00:00:00 2001 From: Gevarist <98388264+Gevarist@users.noreply.github.com> Date: Wed, 23 Aug 2023 13:02:54 +0530 Subject: [PATCH] fix: sherlock --- contracts/ArrakisV2.sol | 21 +++---- contracts/ArrakisV2Resolver.sol | 10 ++++ contracts/abstract/ArrakisV2Storage.sol | 76 +++++++++++++++++++++++-- contracts/libraries/Underlying.sol | 2 +- hardhat.config.ts | 2 +- 5 files changed, 90 insertions(+), 21 deletions(-) diff --git a/contracts/ArrakisV2.sol b/contracts/ArrakisV2.sol index 94dd4d5..710ff8d 100644 --- a/contracts/ArrakisV2.sol +++ b/contracts/ArrakisV2.sol @@ -22,7 +22,6 @@ import {Position} from "./libraries/Position.sol"; import {Pool} from "./libraries/Pool.sol"; import {Underlying as UnderlyingHelper} from "./libraries/Underlying.sol"; import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; -import {hundredPercent} from "./constants/CArrakisV2.sol"; /// @title ArrakisV2 LP vault version 2 /// @notice Smart contract managing liquidity providing strategy for a given token pair @@ -140,12 +139,13 @@ contract ArrakisV2 is IUniswapV3MintCallback, ArrakisV2Storage { range.lowerTick, range.upperTick ); - if (liquidity == 0) continue; liquidity = SafeCast.toUint128( FullMath.mulDiv(liquidity, mintAmount_, ts) ); + if (liquidity == 0) continue; + pool.mint(me, range.lowerTick, range.upperTick, liquidity, ""); } } @@ -183,12 +183,13 @@ contract ArrakisV2 is IUniswapV3MintCallback, ArrakisV2Storage { range.lowerTick, range.upperTick ); - if (liquidity == 0) continue; liquidity = SafeCast.toUint128( FullMath.mulDiv(liquidity, burnAmount_, ts) ); + if (liquidity == 0) continue; + Withdraw memory withdraw = _withdraw( pool, range.lowerTick, @@ -430,21 +431,13 @@ contract ArrakisV2 is IUniswapV3MintCallback, ArrakisV2Storage { liquidity_ ); - (uint256 collect0, uint256 collect1) = pool_.collect( - address(this), + (uint256 collect0, uint256 collect1) = _collectFees( + pool_, lowerTick_, - upperTick_, - type(uint128).max, - type(uint128).max + upperTick_ ); withdraw.fee0 = collect0 - withdraw.burn0; withdraw.fee1 = collect1 - withdraw.burn1; } - - function _applyFees(uint256 fee0_, uint256 fee1_) internal { - uint16 mManagerFeeBPS = managerFeeBPS; - managerBalance0 += (fee0_ * mManagerFeeBPS) / hundredPercent; - managerBalance1 += (fee1_ * mManagerFeeBPS) / hundredPercent; - } } diff --git a/contracts/ArrakisV2Resolver.sol b/contracts/ArrakisV2Resolver.sol index b0de113..a439c79 100644 --- a/contracts/ArrakisV2Resolver.sol +++ b/contracts/ArrakisV2Resolver.sol @@ -155,6 +155,16 @@ contract ArrakisV2Resolver is IArrakisV2Resolver { uint256 mintAmount ) { + /// @dev to not exceed max amount sent by user. + uint256 numberOfRanges = vaultV2_.getRanges().length; + + require( + numberOfRanges < amount0Max_ && numberOfRanges < amount1Max_, + "max amounts should be higher than the number of ranges" + ); + amount0Max_ = amount0Max_ - numberOfRanges; + amount1Max_ = amount1Max_ - numberOfRanges; + uint256 totalSupply = vaultV2_.totalSupply(); UnderlyingPayload memory underlyingPayload = UnderlyingPayload({ ranges: vaultV2_.getRanges(), diff --git a/contracts/abstract/ArrakisV2Storage.sol b/contracts/abstract/ArrakisV2Storage.sol index e4ab6c0..c361da7 100644 --- a/contracts/abstract/ArrakisV2Storage.sol +++ b/contracts/abstract/ArrakisV2Storage.sol @@ -4,6 +4,9 @@ pragma solidity 0.8.13; import { IUniswapV3Factory } from "@uniswap/v3-core/contracts/interfaces/IUniswapV3Factory.sol"; +import { + IUniswapV3Pool +} from "@uniswap/v3-core/contracts/interfaces/IUniswapV3Pool.sol"; import { IERC20, SafeERC20 @@ -21,6 +24,7 @@ import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {Range, Rebalance, InitializePayload} from "../structs/SArrakisV2.sol"; +import {hundredPercent} from "../constants/CArrakisV2.sol"; /// @title ArrakisV2Storage base contract containing all ArrakisV2 storage variables. // solhint-disable-next-line max-states-count @@ -131,11 +135,12 @@ abstract contract ArrakisV2Storage is __ReentrancyGuard_init(); _addPools(params_.feeTiers, params_.token0, params_.token1); - _whitelistRouters(params_.routers); token0 = IERC20(params_.token0); token1 = IERC20(params_.token1); + _whitelistRouters(params_.routers); + _transferOwnership(params_.owner); manager = params_.manager; @@ -206,7 +211,8 @@ abstract contract ArrakisV2Storage is /// @notice set manager /// @param manager_ manager address. /// @dev only callable by owner. - function setManager(address manager_) external onlyOwner { + function setManager(address manager_) external onlyOwner nonReentrant { + _collectFeesOnPools(); _withdrawManagerBalance(); manager = manager_; emit LogSetManager(manager_); @@ -215,8 +221,13 @@ abstract contract ArrakisV2Storage is /// @notice set manager fee bps /// @param managerFeeBPS_ manager fee in basis points. /// @dev only callable by manager. - function setManagerFeeBPS(uint16 managerFeeBPS_) external onlyManager { + function setManagerFeeBPS(uint16 managerFeeBPS_) + external + onlyManager + nonReentrant + { require(managerFeeBPS_ <= 10000, "MFO"); + _collectFeesOnPools(); managerFeeBPS = managerFeeBPS_; emit LogSetManagerFeeBPS(managerFeeBPS_); } @@ -277,12 +288,21 @@ abstract contract ArrakisV2Storage is managerBalance0 = 0; managerBalance1 = 0; + /// @dev token can blacklist manager and make this function fail, + /// so we use try catch to deal with blacklisting. + if (amount0 > 0) { - token0.safeTransfer(manager, amount0); + // solhint-disable-next-line no-empty-blocks + try token0.transfer(manager, amount0) {} catch { + amount0 = 0; + } } if (amount1 > 0) { - token1.safeTransfer(manager, amount1); + // solhint-disable-next-line no-empty-blocks + try token1.transfer(manager, amount1) {} catch { + amount1 = 0; + } } emit LogWithdrawManagerBalance(amount0, amount1); @@ -308,6 +328,46 @@ abstract contract ArrakisV2Storage is } } + function _collectFeesOnPools() internal { + uint256 fees0; + uint256 fees1; + for (uint256 i; i < _ranges.length; i++) { + Range memory range = _ranges[i]; + IUniswapV3Pool pool = IUniswapV3Pool( + factory.getPool(address(token0), address(token1), range.feeTier) + ); + + /// @dev to update the position and collect fees. + pool.burn(range.lowerTick, range.upperTick, 0); + + (uint256 collect0, uint256 collect1) = _collectFees( + pool, + range.lowerTick, + range.upperTick + ); + + fees0 += collect0; + fees1 += collect1; + } + + _applyFees(fees0, fees1); + emit LogCollectedFees(fees0, fees1); + } + + function _collectFees( + IUniswapV3Pool pool_, + int24 lowerTick_, + int24 upperTick_ + ) internal returns (uint256 collect0, uint256 collect1) { + (collect0, collect1) = pool_.collect( + address(this), + lowerTick_, + upperTick_, + type(uint128).max, + type(uint128).max + ); + } + function _whitelistRouters(address[] calldata routers_) internal { for (uint256 i = 0; i < routers_.length; i++) { require( @@ -323,5 +383,11 @@ abstract contract ArrakisV2Storage is emit LogWhitelistRouters(routers_); } + function _applyFees(uint256 fee0_, uint256 fee1_) internal { + uint16 mManagerFeeBPS = managerFeeBPS; + managerBalance0 += (fee0_ * mManagerFeeBPS) / hundredPercent; + managerBalance1 += (fee1_ * mManagerFeeBPS) / hundredPercent; + } + // #endregion internal functions } diff --git a/contracts/libraries/Underlying.sol b/contracts/libraries/Underlying.sol index 2b9114e..e9a3ce1 100644 --- a/contracts/libraries/Underlying.sol +++ b/contracts/libraries/Underlying.sol @@ -308,7 +308,7 @@ library Underlying { if (sqrtRatioAX96 > sqrtRatioBX96) (sqrtRatioAX96, sqrtRatioBX96) = (sqrtRatioBX96, sqrtRatioAX96); - if (sqrtRatioX96 <= sqrtRatioAX96) { + if (sqrtRatioX96 < sqrtRatioAX96) { amount0 = SafeCast.toUint256( SqrtPriceMath.getAmount0Delta( sqrtRatioAX96, diff --git a/hardhat.config.ts b/hardhat.config.ts index b8e47fd..6be9ef7 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -97,7 +97,7 @@ const config: HardhatUserConfig = { { version: "0.8.13", settings: { - optimizer: { enabled: true, runs: 1299 }, + optimizer: { enabled: true, runs: 833 }, }, }, ],