The deposit / withdraw / trade transaction lack of expiration timestamp check and slippage control
The deposit / withdraw / trade transaction lack of expiration timestamp and slippage control\\nLet us look into the heavily forked Uniswap V2 contract addLiquidity function implementation\\n```\\n// **** ADD LIQUIDITY ****\\nfunction _addLiquidity(\\n address tokenA,\\n address tokenB,\\n uint amountADesired,\\n uint amountBDesired,\\n uint amountAMin,\\n uint amountBMin\\n) internal virtual returns (uint amountA, uint amountB) {\\n // create the pair if it doesn't exist yet\\n if (IUniswapV2Factory(factory).getPair(tokenA, tokenB) == address(0)) {\\n  IUniswapV2Factory(factory).createPair(tokenA, tokenB);\\n }\\n (uint reserveA, uint reserveB) = UniswapV2Library.getReserves(factory, tokenA, tokenB);\\n if (reserveA == 0 && reserveB == 0) {\\n  (amountA, amountB) = (amountADesired, amountBDesired);\\n } else {\\n  uint amountBOptimal = UniswapV2Library.quote(amountADesired, reserveA, reserveB);\\n  if (amountBOptimal <= amountBDesired) {\\n   require(amountBOptimal >= amountBMin, 'UniswapV2Router: INSUFFICIENT_B_AMOUNT');\\n   (amountA, amountB) = (amountADesired, amountBOptimal);\\n  } else {\\n   uint amountAOptimal = UniswapV2Library.quote(amountBDesired, reserveB, reserveA);\\n   assert(amountAOptimal <= amountADesired);\\n   require(amountAOptimal >= amountAMin, 'UniswapV2Router: INSUFFICIENT_A_AMOUNT');\\n   (amountA, amountB) = (amountAOptimal, amountBDesired);\\n  }\\n }\\n}\\n\\nfunction addLiquidity(\\n address tokenA,\\n address tokenB,\\n uint amountADesired,\\n uint amountBDesired,\\n uint amountAMin,\\n uint amountBMin,\\n address to,\\n uint deadline\\n) external virtual override ensure(deadline) returns (uint amountA, uint amountB, uint liquidity) {\\n (amountA, amountB) = _addLiquidity(tokenA, tokenB, amountADesired, amountBDesired, amountAMin, amountBMin);\\n address pair = UniswapV2Library.pairFor(factory, tokenA, tokenB);\\n TransferHelper.safeTransferFrom(tokenA, msg.sender, pair, amountA);\\n TransferHelper.safeTransferFrom(tokenB, msg.sender, pair, amountB);\\n liquidity = IUniswapV2Pair(pair).mint(to);\\n}\\n```\\n\\nthe implementation has two point that worth noting,\\nthe first point is the deadline check\\n```\\nmodifier ensure(uint deadline) {\\n require(deadline >= block.timestamp, 'UniswapV2Router: EXPIRED');\\n _;\\n}\\n```\\n\\nThe transaction can be pending in mempool for a long and the trading activity is very time senstive. Without deadline check, the trade transaction can be executed in a long time after the user submit the transaction, at that time, the trade can be done in a sub-optimal price, which harms user's position.\\nThe deadline check ensure that the transaction can be executed on time and the expired transaction revert.\\nthe second point is the slippage control:\\n```\\nrequire(amountAOptimal >= amountAMin, 'UniswapV2Router: INSUFFICIENT_A_AMOUNT');\\n```\\n\\nand\\n```\\nrequire(amountBOptimal >= amountBMin, 'UniswapV2Router: INSUFFICIENT_B_AMOUNT');\\n```\\n\\nthe slippage control the user can receive the least optimal amount of the token they want to trade.\\nIn the current implementation, neither the deadline check nor the slippage control is in place when user deposit / withdraw / trade.
Issue The deposit / withdraw / trade transaction lack of expiration timestamp check and slippage control\\nWe recommend the protocol add deadline check and add slippage control.
According to the whitepaper:\\nDeposits in the highest priced buckets offer the highest valuations on collateral, and hence offer the most liquidity to borrowers. They are also the first buckets that could be used to purchase collateral if a loan were to be liquidated (see 7.0 LIQUIDATIONS). We can think of a bucket's deposit as being utilized if the sum of all deposits in buckets priced higher than it is less than the total debt of all borrowers in the pool. The lowest price among utilized buckets or “lowest utilized price” is called the LUP. If we were to pair off lenders with borrowers, matching the highest priced lenders' deposits with the borrowers' debts in equal quantities, the LUP would be the price of the marginal (lowest priced and therefore least aggressive) lender thus matched (usually, there would be a surplus of lenders that were not matched, corresponding to less than 100% utilization of the pool).\\nThe LUP plays a critical role in Ajna: a borrower who is undercollateralized with respect to the LUP (i.e. with respect to the marginal utilized lender) is eligible for liquidation. Conversely, a lender cannot withdraw deposit if doing so would move the LUP down so far as to make some active loans eligible for liquidation. In order to withdraw quote token in this situation, the lender must first kick the loans in question.\\nBecause the deadline check is missing,\\nAfter a lender submit a transaction and want to add the token into Highest price busket to make sure the quote token can be borrowed out and generate yield.\\nHowever, the transaction is pending in the mempool for a very long time.\\nBorrower create more debt and other lender's add and withdraw quote token before the lender's transaction is executed.\\nAfter a long time later, the lender's transaction is executed.\\nThe lender find out that the highest priced bucket moved and the lender cannot withdraw his token because doing would move the LUP down eligible for liquidiation.\\nAccording to the whitepaper:\\n6.1 Trading collateral for quote token\\nDavid owns 1 ETH, and would like to sell it for 1100 DAI. He puts the 1 ETH into the 1100 bucket as claimable collateral (alongside Carol's 20000 deposit), minting 1100 in LPB in return. He can then redeem that 1100 LPB for quote token, withdrawing 1100 DAI. Note: after David's withdrawal, the LUP remains at 1100. If the book were different such that his withdrawal would move the LUP below Bob's threshold price of 901.73, he would not be able to withdraw all of the DAI.\\nThe case above is ideal, however, because the deadline check is missing, and there is no slippage control, the transactoin can be pending for a long time and by the time the trade transaction is lended, the withdraw amount can be less than 1100 DAI.\\nAnother example for lack of slippage, for example, the function below is called:\\n```\\n/// @inheritdoc IPoolLenderActions\\nfunction removeQuoteToken(\\n uint256 maxAmount_,\\n uint256 index_\\n) external override nonReentrant returns (uint256 removedAmount_, uint256 redeemedLPs_) {\\n _revertIfAuctionClearable(auctions, loans);\\n\\n PoolState memory poolState = _accruePoolInterest();\\n\\n _revertIfAuctionDebtLocked(deposits, poolBalances, index_, poolState.inflator);\\n\\n uint256 newLup;\\n (\\n  removedAmount_,\\n  redeemedLPs_,\\n  newLup\\n ) = LenderActions.removeQuoteToken(\\n  buckets,\\n  deposits,\\n  poolState,\\n  RemoveQuoteParams({\\n   maxAmount:      maxAmount_,\\n   index:          index_,\\n   thresholdPrice: Loans.getMax(loans).thresholdPrice\\n  })\\n );\\n\\n // update pool interest rate state\\n _updateInterestState(poolState, newLup);\\n\\n // move quote token amount from pool to lender\\n _transferQuoteToken(msg.sender, removedAmount_);\\n}\\n```\\n\\nwithout specificing the minReceived amount, the removedAmount can be very small comparing to the maxAmount user speicifced.
```\\n// **** ADD LIQUIDITY ****\\nfunction _addLiquidity(\\n address tokenA,\\n address tokenB,\\n uint amountADesired,\\n uint amountBDesired,\\n uint amountAMin,\\n uint amountBMin\\n) internal virtual returns (uint amountA, uint amountB) {\\n // create the pair if it doesn't exist yet\\n if (IUniswapV2Factory(factory).getPair(tokenA, tokenB) == address(0)) {\\n  IUniswapV2Factory(factory).createPair(tokenA, tokenB);\\n }\\n (uint reserveA, uint reserveB) = UniswapV2Library.getReserves(factory, tokenA, tokenB);\\n if (reserveA == 0 && reserveB == 0) {\\n  (amountA, amountB) = (amountADesired, amountBDesired);\\n } else {\\n  uint amountBOptimal = UniswapV2Library.quote(amountADesired, reserveA, reserveB);\\n  if (amountBOptimal <= amountBDesired) {\\n   require(amountBOptimal >= amountBMin, 'UniswapV2Router: INSUFFICIENT_B_AMOUNT');\\n   (amountA, amountB) = (amountADesired, amountBOptimal);\\n  } else {\\n   uint amountAOptimal = UniswapV2Library.quote(amountBDesired, reserveB, reserveA);\\n   assert(amountAOptimal <= amountADesired);\\n   require(amountAOptimal >= amountAMin, 'UniswapV2Router: INSUFFICIENT_A_AMOUNT');\\n   (amountA, amountB) = (amountAOptimal, amountBDesired);\\n  }\\n }\\n}\\n\\nfunction addLiquidity(\\n address tokenA,\\n address tokenB,\\n uint amountADesired,\\n uint amountBDesired,\\n uint amountAMin,\\n uint amountBMin,\\n address to,\\n uint deadline\\n) external virtual override ensure(deadline) returns (uint amountA, uint amountB, uint liquidity) {\\n (amountA, amountB) = _addLiquidity(tokenA, tokenB, amountADesired, amountBDesired, amountAMin, amountBMin);\\n address pair = UniswapV2Library.pairFor(factory, tokenA, tokenB);\\n TransferHelper.safeTransferFrom(tokenA, msg.sender, pair, amountA);\\n TransferHelper.safeTransferFrom(tokenB, msg.sender, pair, amountB);\\n liquidity = IUniswapV2Pair(pair).mint(to);\\n}\\n```\\n