Sherlock 39: expiration timestamp and slippage control

.gitignore

@@ -13,4 +13,5 @@ reports/
 coverage/
 *.info
 report/
-keystore/
+keystore/
+broadcast/

src/ERC20Pool.sol

@@ -38,7 +38,10 @@ import {
  _roundToScale,
  _roundUpToScale
 } from './libraries/helpers/PoolHelper.sol';
-import { _revertIfAuctionClearable } from './libraries/helpers/RevertsHelper.sol';
+import { 
+ _revertIfAuctionClearable,
+ _revertOnExpiry 
+} from './libraries/helpers/RevertsHelper.sol';
 
 import { Loans } from './libraries/internal/Loans.sol';
 import { Deposits } from './libraries/internal/Deposits.sol';
@@ -184,7 +187,8 @@ contract ERC20Pool is FlashloanablePool, IERC20Pool {
  address borrowerAddress_,
  uint256 maxQuoteTokenAmountToRepay_,
  uint256 collateralAmountToPull_,
- address collateralReceiver_
+ address collateralReceiver_,
+ uint256 limitIndex_
  ) external nonReentrant {
  PoolState memory poolState = _accruePoolInterest();
 
@@ -200,7 +204,8 @@ contract ERC20Pool is FlashloanablePool, IERC20Pool {
  poolState,
  borrowerAddress_,
  maxQuoteTokenAmountToRepay_,
- collateralAmountToPull_
+ collateralAmountToPull_,
+ limitIndex_
  );
 
  emit RepayDebt(borrowerAddress_, result.quoteTokenToRepay, collateralAmountToPull_, result.newLup);
@@ -242,8 +247,10 @@ contract ERC20Pool is FlashloanablePool, IERC20Pool {
  */
  function addCollateral(
  uint256 amountToAdd_,
- uint256 index_
+ uint256 index_,
+ uint256 expiry_
  ) external override nonReentrant returns (uint256 bucketLPs_) {
+ _revertOnExpiry(expiry_);
  PoolState memory poolState = _accruePoolInterest();
 
  // revert if the dust amount was not exceeded, but round on the scale amount

src/ERC721Pool.sol

@@ -27,7 +27,10 @@ import { IERC721Taker } from './interfaces/pool/erc721/IERC721Taker.sol'
 
 import { FlashloanablePool } from './base/FlashloanablePool.sol';
 
-import { _revertIfAuctionClearable } from './libraries/helpers/RevertsHelper.sol';
+import { 
+ _revertIfAuctionClearable,
+ _revertOnExpiry 
+} from './libraries/helpers/RevertsHelper.sol';
 
 import { Maths } from './libraries/internal/Maths.sol';
 import { Deposits } from './libraries/internal/Deposits.sol';
@@ -189,7 +192,8 @@ contract ERC721Pool is FlashloanablePool, IERC721Pool {
  address borrowerAddress_,
  uint256 maxQuoteTokenAmountToRepay_,
  uint256 noOfNFTsToPull_,
- address collateralReceiver_
+ address collateralReceiver_,
+ uint256 limitIndex_
  ) external nonReentrant {
  PoolState memory poolState = _accruePoolInterest();
 
@@ -201,7 +205,8 @@ contract ERC721Pool is FlashloanablePool, IERC721Pool {
  poolState,
  borrowerAddress_,
  maxQuoteTokenAmountToRepay_,
- Maths.wad(noOfNFTsToPull_)
+ Maths.wad(noOfNFTsToPull_),
+ limitIndex_
  );
 
  emit RepayDebt(borrowerAddress_, result.quoteTokenToRepay, noOfNFTsToPull_, result.newLup);
@@ -245,8 +250,10 @@ contract ERC721Pool is FlashloanablePool, IERC721Pool {
  */
  function addCollateral(
  uint256[] calldata tokenIdsToAdd_,
- uint256 index_
+ uint256 index_,
+ uint256 expiry_
  ) external override nonReentrant returns (uint256 bucketLPs_) {
+ _revertOnExpiry(expiry_);
  PoolState memory poolState = _accruePoolInterest();
 
  bucketLPs_ = LenderActions.addCollateral(

src/PositionManager.sol

@@ -252,7 +252,8 @@ contract PositionManager is ERC721, PermitERC721, IPositionManager, Multicall, R
  ) = IPool(params_.pool).moveQuoteToken(
  maxQuote,
  params_.fromIndex,
- params_.toIndex
+ params_.toIndex,
+ params_.expiry
  );
 
  // update position LPs state

src/base/Pool.sol

@@ -45,7 +45,8 @@ import {
 } from '../libraries/helpers/PoolHelper.sol';
 import {
  _revertIfAuctionDebtLocked,
- _revertIfAuctionClearable
+ _revertIfAuctionClearable,
+ _revertOnExpiry
 } from '../libraries/helpers/RevertsHelper.sol';
 
 import { Buckets } from '../libraries/internal/Buckets.sol';
@@ -131,8 +132,10 @@ abstract contract Pool is Clone, ReentrancyGuard, Multicall, IPool {
  /// @inheritdoc IPoolLenderActions
  function addQuoteToken(
  uint256 quoteTokenAmountToAdd_,
- uint256 index_
+ uint256 index_,
+ uint256 expiry_
  ) external override nonReentrant returns (uint256 bucketLPs_) {
+ _revertOnExpiry(expiry_);
  PoolState memory poolState = _accruePoolInterest();
 
  // round to token precision
@@ -173,8 +176,10 @@ abstract contract Pool is Clone, ReentrancyGuard, Multicall, IPool {
  function moveQuoteToken(
  uint256 maxAmountToMove_,
  uint256 fromIndex_,
- uint256 toIndex_
+ uint256 toIndex_,
+ uint256 expiry_
  ) external override nonReentrant returns (uint256 fromBucketLPs_, uint256 toBucketLPs_) {
+ _revertOnExpiry(expiry_);
  PoolState memory poolState = _accruePoolInterest();
 
  _revertIfAuctionDebtLocked(deposits, poolBalances, fromIndex_, poolState.inflator);

src/interfaces/pool/commons/IPoolErrors.sol

@@ -184,6 +184,11 @@ interface IPoolErrors {
  */
  error TakeNotPastCooldown();
 
+ /**
+ * @notice Current block timestamp has reached or exceeded a user-provided expiration.
+ */
+ error TransactionExpired();
+
  /**
  * @notice Owner of the LP tokens attemps to transfer LPs to same address.
  */

src/interfaces/pool/commons/IPoolLenderActions.sol

@@ -10,11 +10,13 @@ interface IPoolLenderActions {
  * @notice Called by lenders to add an amount of credit at a specified price bucket.
  * @param amount The amount of quote token to be added by a lender.
  * @param index The index of the bucket to which the quote tokens will be added.
+ * @param expiry Timestamp after which this TX will revert, preventing inclusion in a block with unfavorable price.
  * @return lpbChange The amount of LP Tokens changed for the added quote tokens.
  */
  function addQuoteToken(
  uint256 amount,
- uint256 index
+ uint256 index,
+ uint256 expiry
  ) external returns (uint256 lpbChange);
 
  /**
@@ -35,13 +37,15 @@ interface IPoolLenderActions {
  * @param maxAmount The maximum amount of quote token to be moved by a lender.
  * @param fromIndex The bucket index from which the quote tokens will be removed.
  * @param toIndex The bucket index to which the quote tokens will be added.
+ * @param expiry Timestamp after which this TX will revert, preventing inclusion in a block with unfavorable price.
  * @return lpbAmountFrom The amount of LPs moved out from bucket.
  * @return lpbAmountTo The amount of LPs moved to destination bucket.
  */
  function moveQuoteToken(
  uint256 maxAmount,
  uint256 fromIndex,
- uint256 toIndex
+ uint256 toIndex,
+ uint256 expiry
  ) external returns (uint256 lpbAmountFrom, uint256 lpbAmountTo);
 
  /**

src/interfaces/pool/erc20/IERC20PoolBorrowerActions.sol

@@ -30,11 +30,13 @@ interface IERC20PoolBorrowerActions {
  * @param maxQuoteTokenAmountToRepay_ The amount of quote tokens to repay.
  * @param collateralAmountToPull_ The amount of collateral to be puled from the pool.
  * @param recipient_ The address to receive amount of pulled collateral.
+ * @param limitIndex_ Ensures LUP has not moved far from state when borrower pulls collateral.
  */
  function repayDebt(
  address borrowerAddress_,
  uint256 maxQuoteTokenAmountToRepay_,
  uint256 collateralAmountToPull_,
- address recipient_
+ address recipient_,
+ uint256 limitIndex_
  ) external;
 }

src/interfaces/pool/erc20/IERC20PoolLenderActions.sol

@@ -9,11 +9,14 @@ interface IERC20PoolLenderActions {
 
  /**
  * @notice Deposit claimable collateral into a specified bucket.
- * @param amount Amount of collateral to deposit.
- * @param index The bucket index to which collateral will be deposited.
+ * @param amount Amount of collateral to deposit.
+ * @param index The bucket index to which collateral will be deposited.
+ * @param expiry Timestamp after which this TX will revert, preventing inclusion in a block with unfavorable price.
+ * @return lpbChange The amount of LP Tokens changed for the added collateral.
  */
  function addCollateral(
  uint256 amount,
- uint256 index
+ uint256 index,
+ uint256 expiry
  ) external returns (uint256 lpbChange);
 }

src/interfaces/pool/erc721/IERC721PoolBorrowerActions.sol

@@ -30,11 +30,13 @@ interface IERC721PoolBorrowerActions {
  * @param maxQuoteTokenAmountToRepay_ The amount of quote tokens to repay.
  * @param noOfNFTsToPull_ The integer number of NFT collateral to be puled from the pool.
  * @param recipient_ The address to receive amount of pulled collateral.
+ * @param limitIndex_ Ensures LUP has not moved far from state when borrower pulls collateral.
  */
  function repayDebt(
  address borrowerAddress_,
  uint256 maxQuoteTokenAmountToRepay_,
  uint256 noOfNFTsToPull_,
- address recipient_
+ address recipient_,
+ uint256 limitIndex_
  ) external;
 }

src/interfaces/pool/erc721/IERC721PoolLenderActions.sol

@@ -11,10 +11,13 @@ interface IERC721PoolLenderActions {
  * @notice Deposit claimable collateral into a specified bucket.
  * @param tokenIds Array of collateral to deposit.
  * @param index The bucket index to which collateral will be deposited.
+ * @param expiry Timestamp after which this TX will revert, preventing inclusion in a block with unfavorable price.
+ * @return lpbChange The amount of LP Tokens changed for the added collateral.
  */
  function addCollateral(
  uint256[] calldata tokenIds,
- uint256 index
+ uint256 index,
+ uint256 expiry
  ) external returns (uint256);
 
  /**

src/interfaces/position/IPositionManagerOwnerActions.sol

@@ -95,6 +95,7 @@ interface IPositionManagerOwnerActions {
  address pool; // The pool address associated with positions NFT
  uint256 fromIndex; // The bucket index from which liquidity should be moved
  uint256 toIndex; // The bucket index to which liquidity should be moved
+ uint256 expiry; // Timestamp after which this TX will revert, preventing inclusion in a block with unfavorable price
  }
 
  /**

src/libraries/external/BorrowerActions.sol

@@ -20,7 +20,10 @@ import {
  _priceAt,
  _isCollateralized
 } from '../helpers/PoolHelper.sol';
-import { _revertOnMinDebt } from '../helpers/RevertsHelper.sol';
+import { 
+ _revertIfLupDroppedBelowLimit, 
+ _revertOnMinDebt
+} from '../helpers/RevertsHelper.sol';
 
 import { Buckets } from '../internal/Buckets.sol';
 import { Deposits } from '../internal/Deposits.sol';
@@ -200,11 +203,13 @@ library BorrowerActions {
 
  // determine new lup index and revert if borrow happens at a price higher than the specified limit (lower index than lup index)
  vars.lupId = _lupIndex(deposits_, result_.poolDebt);
- if (vars.lupId > limitIndex_) revert LimitIndexReached();
+ result_.newLup = _priceAt(vars.lupId);
+ // if (vars.lupId > limitIndex_) revert LimitIndexReached();
+
+ _revertIfLupDroppedBelowLimit(result_.newLup, limitIndex_);
 
  // calculate new lup and check borrow action won't push borrower into a state of under-collateralization
  // this check also covers the scenario when loan is already auctioned
- result_.newLup = _priceAt(vars.lupId);
 
  if (!_isCollateralized(vars.borrowerDebt, borrower.collateral, result_.newLup, poolState_.poolType)) {
  revert BorrowerUnderCollateralized();
@@ -254,6 +259,7 @@ library BorrowerActions {
  * - borrower debt less than pool min debt AmountLTMinDebt()
  * - borrower not sender BorrowerNotSender()
  * - not enough collateral to pull InsufficientCollateral()
+ * - limit price reached LimitIndexReached()
  * @dev emit events:
  * - Auctions._settleAuction:
  * - AuctionNFTSettle or AuctionSettle
@@ -266,7 +272,8 @@ library BorrowerActions {
  PoolState calldata poolState_,
  address borrowerAddress_,
  uint256 maxQuoteTokenAmountToRepay_,
- uint256 collateralAmountToPull_
+ uint256 collateralAmountToPull_,
+ uint256 limitIndex_
  ) external returns (
  RepayDebtResult memory result_
  ) {
@@ -356,6 +363,8 @@ library BorrowerActions {
  // calculate LUP only if it wasn't calculated in repay action
  if (!vars.repay) result_.newLup = _lup(deposits_, result_.poolDebt);
 
+ _revertIfLupDroppedBelowLimit(result_.newLup, limitIndex_);
+
  uint256 encumberedCollateral = borrower.t0Debt != 0 ? Maths.wdiv(vars.borrowerDebt, result_.newLup) : 0;
 
  if (borrower.collateral - encumberedCollateral < collateralAmountToPull_) revert InsufficientCollateral();

src/libraries/helpers/RevertsHelper.sol

@@ -10,7 +10,7 @@ import {
  PoolBalancesState
 } from '../../interfaces/pool/commons/IPoolState.sol';
 
-import { _minDebtAmount } from './PoolHelper.sol';
+import { _minDebtAmount, _priceAt } from './PoolHelper.sol';
 
 import { Loans } from '../internal/Loans.sol';
 import { Deposits } from '../internal/Deposits.sol';
@@ -20,7 +20,9 @@ import { Maths } from '../internal/Maths.sol';
  error AuctionNotCleared();
  error AmountLTMinDebt();
  error DustAmountNotExceeded();
+ error LimitIndexReached();
  error RemoveDepositLockedByAuctionDebt();
+ error TransactionExpired();
 
  /**
  * @notice Called by LPB removal functions assess whether or not LPB is locked.
@@ -58,6 +60,30 @@ import { Maths } from '../internal/Maths.sol';
  }
  }
 
+ /**
+ * @notice Check if LUP is at or above index limit provided by borrower.
+ * @notice Prevents stale transactions and certain MEV manipulations.
+ * @param newLup_ New LUP as a result of the borrower action.
+ * @param limitIndex_ Price limit provided by user creating the TX.
+ */
+ function _revertIfLupDroppedBelowLimit(
+ uint256 newLup_,
+ uint256 limitIndex_
+ ) pure {
+ if (newLup_ < _priceAt(limitIndex_)) revert LimitIndexReached();
+ }
+
+ /**
+ * @notice Check if expiration provided by user has met or exceeded current block height timestamp.
+ * @notice Prevents stale transactions interacting with the pool at potentially unfavorable prices.
+ * @param expiry_ Expiration provided by user when creating the TX.
+ */
+ function _revertOnExpiry(
+ uint256 expiry_
+ ) view {
+ if (block.timestamp >= expiry_) revert TransactionExpired();
+ }
+
  /**
  * @notice Called when borrower debt changes, ensuring minimum debt rules are honored.
  * @param loans_ Loans heap, used to determine loan count.