Skip to content

Commit

Permalink
fix : audits
Browse files Browse the repository at this point in the history 
* fix: add max manager fee checks

* fix: AR-213 first minter can change LP token pricing

* fix: AR-214 and AR-215

* chore: AR 224 add getRanges

* chore: remove manager interface call

* fix: AR 225 check liquidity is zero

* chore: final clean up

Co-authored-by: κασσάνδρα.eth <[email protected]>
  • Loading branch information
@Gevarist @kassandraoftroy
Gevarist and kassandraoftroy committed Dec 2, 2022
1 parent f1e174e commit e49c5da
Show file tree
Hide file tree
Showing 23 changed files with 161 additions and 334 deletions.
115 changes: 55 additions & 60 deletions contracts/ArrakisV2.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,15 @@ import {
} from "./structs/SArrakisV2.sol";
import {Position} from "./libraries/Position.sol";
import {Pool} from "./libraries/Pool.sol";
import {Manager} from "./libraries/Manager.sol";
import {Underlying as UnderlyingHelper} from "./libraries/Underlying.sol";

/// @dev DO NOT ADD STATE VARIABLES - APPEND THEM TO ArrakisV2Storage
contract ArrakisV2 is IUniswapV3MintCallback, ArrakisV2Storage {
using SafeERC20 for IERC20;
using EnumerableSet for EnumerableSet.AddressSet;

constructor(IUniswapV3Factory factory_, address arrakisTreasury_)
ArrakisV2Storage(factory_, arrakisTreasury_)
{} // solhint-disable-line no-empty-blocks
// solhint-disable-next-line no-empty-blocks
constructor(IUniswapV3Factory factory_) ArrakisV2Storage(factory_) {}

/// @notice Uniswap V3 callback fn, called back on pool.mint
function uniswapV3MintCallback(
Expand All @@ -61,12 +59,13 @@ contract ArrakisV2 is IUniswapV3MintCallback, ArrakisV2Storage {
);
address me = address(this);
uint256 totalSupply = totalSupply();
bool isTotalSupplyGtZero = totalSupply > 0;
(
uint256 current0,
uint256 current1,
uint256 fee0,
uint256 fee1
) = totalSupply > 0
) = isTotalSupplyGtZero
? UnderlyingHelper.totalUnderlyingWithFees(
UnderlyingPayload({
ranges: ranges,
Expand All @@ -77,12 +76,35 @@ contract ArrakisV2 is IUniswapV3MintCallback, ArrakisV2Storage {
})
)
: (init0, init1, 0, 0);
uint256 denominator = totalSupply > 0 ? totalSupply : 1 ether;
/// @dev current0 and current1 include fees and left over (but not admin balances)
uint256 denominator = isTotalSupplyGtZero ? totalSupply : 1 ether;

/// @dev current0 and current1 include fees and left over (but not manager balances)
amount0 = FullMath.mulDivRoundingUp(mintAmount_, current0, denominator);
amount1 = FullMath.mulDivRoundingUp(mintAmount_, current1, denominator);

// #region check amount0 is a multiple of current0.

if (!isTotalSupplyGtZero) {
uint256 amount0Mint = FullMath.mulDiv(
amount0,
denominator,
current0
);
uint256 amount1Mint = FullMath.mulDiv(
amount1,
denominator,
current1
);

require(
(amount0Mint < amount1Mint ? amount0Mint : amount1Mint) ==
mintAmount_,
"A0&A1"
);
}

// #endregion check amount0 is a multiple of current0.

_mint(receiver_, mintAmount_);

// transfer amounts owed to contract
Expand Down Expand Up @@ -123,10 +145,10 @@ contract ArrakisV2 is IUniswapV3MintCallback, ArrakisV2Storage {
);
underlying.leftOver0 =
token0.balanceOf(address(this)) -
(managerBalance0 + arrakisBalance0);
managerBalance0;
underlying.leftOver1 =
token1.balanceOf(address(this)) -
(managerBalance1 + arrakisBalance1);
managerBalance1;

{
// the proportion of user balance.
Expand Down Expand Up @@ -200,15 +222,21 @@ contract ArrakisV2 is IUniswapV3MintCallback, ArrakisV2Storage {
token1.safeTransfer(receiver_, amount1);
}

// intentional underflow revert if managerBalance > contract's token balance
uint256 leftover0 = token0.balanceOf(address(this)) - managerBalance0;
uint256 leftover1 = token1.balanceOf(address(this)) - managerBalance1;

require(
token0.balanceOf(address(this)) >=
managerBalance0 + arrakisBalance0,
"MB0"
(leftover0 <= underlying.leftOver0) ||
((leftover0 - underlying.leftOver0) <=
FullMath.mulDiv(total.burn0, _burnBuffer, 10000)),
"L0"
);
require(
token1.balanceOf(address(this)) >=
managerBalance1 + arrakisBalance1,
"MB1"
(leftover1 <= underlying.leftOver1) ||
((leftover1 - underlying.leftOver1) <=
FullMath.mulDiv(total.burn1, _burnBuffer, 10000)),
"L1"
);

// For monitoring how much user burn LP token for getting their token back.
Expand All @@ -220,37 +248,28 @@ contract ArrakisV2 is IUniswapV3MintCallback, ArrakisV2Storage {

// solhint-disable-next-line function-max-lines
function rebalance(
Range[] calldata ranges_,
Range[] calldata rangesToAdd_,
Rebalance calldata rebalanceParams_,
Range[] calldata rangesToRemove_
) external onlyManager {
for (uint256 i = 0; i < ranges_.length; i++) {
(bool exist, ) = Position.rangeExist(ranges, ranges_[i]);
for (uint256 i = 0; i < rangesToAdd_.length; i++) {
(bool exist, ) = Position.rangeExist(ranges, rangesToAdd_[i]);
require(!exist, "NRRE");
// check that the pool exist on Uniswap V3.
address pool = factory.getPool(
address(token0),
address(token1),
ranges_[i].feeTier
rangesToAdd_[i].feeTier
);
require(pool != address(0), "NUP");
require(_pools.contains(pool), "P");
// TODO: can reuse the pool got previously.
require(Pool.validateTickSpacing(pool, ranges_[i]), "RTS");
require(Pool.validateTickSpacing(pool, rangesToAdd_[i]), "RTS");

ranges.push(ranges_[i]);
ranges.push(rangesToAdd_[i]);
}
_rebalance(rebalanceParams_);
require(
token0.balanceOf(address(this)) >=
managerBalance0 + arrakisBalance0,
"MB0"
);
require(
token1.balanceOf(address(this)) >=
managerBalance1 + arrakisBalance1,
"MB1"
);
require(token0.balanceOf(address(this)) >= managerBalance0, "MB0");
require(token1.balanceOf(address(this)) >= managerBalance1, "MB1");
for (uint256 i = 0; i < rangesToRemove_.length; i++) {
(bool exist, uint256 index) = Position.rangeExist(
ranges,
Expand Down Expand Up @@ -283,40 +302,22 @@ contract ArrakisV2 is IUniswapV3MintCallback, ArrakisV2Storage {
managerBalance1 = 0;

if (amount0 > 0) {
token0.safeTransfer(address(manager), amount0);
token0.safeTransfer(manager, amount0);
}

if (amount1 > 0) {
token1.safeTransfer(address(manager), amount1);
token1.safeTransfer(manager, amount1);
}

emit LogWithdrawManagerBalance(amount0, amount1);
}

function withdrawArrakisBalance() external {
uint256 amount0 = arrakisBalance0;
uint256 amount1 = arrakisBalance1;

arrakisBalance0 = 0;
arrakisBalance1 = 0;

if (amount0 > 0) {
token0.safeTransfer(arrakisTreasury, amount0);
}

if (amount1 > 0) {
token1.safeTransfer(arrakisTreasury, amount1);
}

emit LogWithdrawArrakisBalance(amount0, amount1);
}

// solhint-disable-next-line function-max-lines, code-complexity
function _rebalance(Rebalance calldata rebalanceParams_)
internal
nonReentrant
{
// Burns
// Burns.
uint256 aggregator0 = 0;
uint256 aggregator1 = 0;
for (uint256 i = 0; i < rebalanceParams_.removes.length; i++) {
Expand Down Expand Up @@ -344,8 +345,7 @@ contract ArrakisV2 is IUniswapV3MintCallback, ArrakisV2Storage {
emit LogCollectedFees(aggregator0, aggregator1);
}

// Swap

// Swap.
if (rebalanceParams_.swap.amountIn > 0) {
{
require(_routers.contains(rebalanceParams_.swap.router), "NR");
Expand Down Expand Up @@ -443,8 +443,6 @@ contract ArrakisV2 is IUniswapV3MintCallback, ArrakisV2Storage {
liquidity_
);

/// @dev relying on return values here means we WILL BREAK with fee-on-transfer tokens
/// we assume this is fine as UniswapV3 is already broken for fee-on-transfer tokens
(uint256 collect0, uint256 collect1) = pool_.collect(
address(this),
lowerTick_,
Expand All @@ -458,10 +456,7 @@ contract ArrakisV2 is IUniswapV3MintCallback, ArrakisV2Storage {
}

function _applyFees(uint256 fee0_, uint256 fee1_) internal {
uint16 managerFeeBPS = Manager.getManagerFeeBPS(manager);
managerBalance0 += (fee0_ * managerFeeBPS) / 10000;
managerBalance1 += (fee1_ * managerFeeBPS) / 10000;
arrakisBalance0 += (fee0_ * arrakisFeeBPS) / 10000;
arrakisBalance1 += (fee1_ * arrakisFeeBPS) / 10000;
}
}
4 changes: 2 additions & 2 deletions contracts/ArrakisV2Factory.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ contract ArrakisV2Factory is ArrakisV2FactoryStorage {
return _append("Arrakis Vault V2 ", symbol0, "/", symbol1);
}

/// @notice numVaults counts the total number of Harvesters in existence
/// @return result total number of Harvesters deployed
/// @notice numVaults counts the total number of vaults in existence
/// @return result total number of vaults deployed
function numVaults() public view returns (uint256 result) {
return _vaults.length();
}
Expand Down
33 changes: 5 additions & 28 deletions contracts/ArrakisV2Helper.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ contract ArrakisV2Helper is IArrakisV2Helper {
returns (UnderlyingOutput memory underlying)
{
UnderlyingPayload memory underlyingPayload = UnderlyingPayload({
ranges: ranges(vault_),
ranges: vault_.getRanges(),
factory: vault_.factory(),
token0: address(vault_.token0()),
token1: address(vault_.token1()),
Expand All @@ -48,12 +48,10 @@ contract ArrakisV2Helper is IArrakisV2Helper {

underlying.leftOver0 =
IERC20(underlyingPayload.token0).balanceOf(underlyingPayload.self) -
IArrakisV2(underlyingPayload.self).managerBalance0() -
IArrakisV2(underlyingPayload.self).arrakisBalance0();
IArrakisV2(underlyingPayload.self).managerBalance0();
underlying.leftOver1 =
IERC20(underlyingPayload.token1).balanceOf(underlyingPayload.self) -
IArrakisV2(underlyingPayload.self).managerBalance1() -
IArrakisV2(underlyingPayload.self).arrakisBalance1();
IArrakisV2(underlyingPayload.self).managerBalance1();
}

function totalUnderlyingWithFees(IArrakisV2 vault_)
Expand All @@ -67,7 +65,7 @@ contract ArrakisV2Helper is IArrakisV2Helper {
)
{
UnderlyingPayload memory underlyingPayload = UnderlyingPayload({
ranges: ranges(vault_),
ranges: vault_.getRanges(),
factory: vault_.factory(),
token0: address(vault_.token0()),
token1: address(vault_.token1()),
Expand All @@ -84,7 +82,7 @@ contract ArrakisV2Helper is IArrakisV2Helper {
returns (uint256 amount0, uint256 amount1)
{
UnderlyingPayload memory underlyingPayload = UnderlyingPayload({
ranges: ranges(vault_),
ranges: vault_.getRanges(),
factory: vault_.factory(),
token0: address(vault_.token0()),
token1: address(vault_.token1()),
Expand Down Expand Up @@ -168,27 +166,6 @@ contract ArrakisV2Helper is IArrakisV2Helper {

// #endregion Rebalance helper functions

function ranges(IArrakisV2 vault_)
public
view
returns (Range[] memory rgs)
{
uint256 index;
while (true) {
try vault_.ranges(index) returns (Range memory) {
index++;
} catch {
break;
}
}

rgs = new Range[](index);

for (uint256 i = 0; i < index; i++) {
rgs[i] = vault_.ranges(i);
}
}

// #region internal functions

function _getAmountsAndFeesFromLiquidity(
Expand Down
16 changes: 7 additions & 9 deletions contracts/ArrakisV2Resolver.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ contract ArrakisV2Resolver is IArrakisV2Resolver {
swapRouter = swapRouter_;
}

// no swapping. Standard rebalance.
// Standard rebalance (without swapping)
// solhint-disable-next-line function-max-lines, code-complexity
function standardRebalance(
RangeWeight[] memory rangeWeights_,
Expand All @@ -59,7 +59,7 @@ contract ArrakisV2Resolver is IArrakisV2Resolver {
address token0Addr;
address token1Addr;
{
Range[] memory ranges = helper.ranges(vaultV2_);
Range[] memory ranges = vaultV2_.getRanges();

token0Addr = address(vaultV2_.token0());
token1Addr = address(vaultV2_.token1());
Expand Down Expand Up @@ -108,8 +108,6 @@ contract ArrakisV2Resolver is IArrakisV2Resolver {
}
}

// TODO check if sum of weight is < 10000

_requireWeightUnder100(rangeWeights_);

rebalanceParams.deposits = new PositionLiquidity[](
Expand Down Expand Up @@ -150,7 +148,7 @@ contract ArrakisV2Resolver is IArrakisV2Resolver {
uint256 totalSupply = vaultV2_.totalSupply();
require(totalSupply > 0, "total supply");

Range[] memory ranges = helper.ranges(vaultV2_);
Range[] memory ranges = vaultV2_.getRanges();

{
UnderlyingOutput memory underlying;
Expand All @@ -170,12 +168,10 @@ contract ArrakisV2Resolver is IArrakisV2Resolver {
);
underlying.leftOver0 =
vaultV2_.token0().balanceOf(address(vaultV2_)) -
vaultV2_.managerBalance0() -
vaultV2_.arrakisBalance0();
vaultV2_.managerBalance0();
underlying.leftOver1 =
vaultV2_.token1().balanceOf(address(vaultV2_)) -
vaultV2_.managerBalance1() -
vaultV2_.arrakisBalance1();
vaultV2_.managerBalance1();

{
uint256 amount0 = FullMath.mulDiv(
Expand Down Expand Up @@ -217,6 +213,8 @@ contract ArrakisV2Resolver is IArrakisV2Resolver {
);
}

if (liquidity == 0) continue;

burns[i] = BurnLiquidity({
liquidity: SafeCast.toUint128(
FullMath.mulDiv(liquidity, amountToBurn_, totalSupply)
Expand Down
4 changes: 4 additions & 0 deletions contracts/__mocks__/ManagerProxyMock.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,8 @@ contract ManagerProxyMock is IManager {
function fundVaultBalance(address vault) external payable {
// empty
}

function setManagerFeeBPS(address vault_, uint16 fees_) external {
IArrakisV2(vault_).setManagerFeeBPS(fees_);
}
}
Loading

0 comments on commit e49c5da

Please sign in to comment.