Sherlock 39: expiration timestamp and slippage control (#600)

* ignore forge script tx broadcasts

* provide LUP limit when pulling collateral

* expiration checks for adding to buckets

* unit tests for addQuote and addCollateral expiration

* expiration checks for moveQuote

* ensure borrower debt exceeds dust amount regardless of loan count

* Revert "ensure borrower debt exceeds dust amount regardless of loan count"

This reverts commit c0a64f9.

* feedback from PR #600 (#604)

* removed comment

* Address review comments, update comment for _revertIfLupDroppedBelowL… (#607)

* Address review comments, update comment for _revertIfLupDroppedBelowLimit, changed LimitIndexReached to LimitIndexExceeded

* Remove lup Id variable, calculate LUP price directly

* Remove unused struct member

---------

Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com>

.gitignore

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

docs/Functions.md

@@ -370,7 +370,7 @@
 	- BorrowerActions.drawDebt()
 		- borrower not sender BorrowerNotSender()
 		- borrower debt less than pool min debt AmountLTMinDebt()
-		- limit price reached LimitIndexReached()
+		- limit price reached LimitIndexExceeded()
 		- borrower cannot draw more debt BorrowerUnderCollateralized()
 
 	emit events:

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

@@ -110,7 +110,7 @@ interface IPoolErrors {
     /**
      *  @notice Borrower is attempting to borrow more quote token than is available before the supplied limitIndex.
      */
-    error LimitIndexReached();
+    error LimitIndexExceeded();
 
     /**
      *  @notice When moving quote token HTP must stay below LUP.
@@ -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';
@@ -47,15 +50,13 @@ library BorrowerActions {
         uint256 t0BorrowAmount;        // [WAD] t0 amount to borrow
         uint256 t0DebtChange;          // [WAD] additional t0 debt resulted from draw debt action
         bool    inAuction;             // true if loan is auctioned
-        uint256 lupId;                 // id of new LUP
         bool    pledge;                // true if pledge action
         bool    stampT0Np;             // true if loan's t0 neutral price should be restamped (when drawing debt or pledge settles auction)
     }
     struct RepayDebtLocalVars {
         uint256 borrowerDebt;          // [WAD] borrower's accrued debt
         uint256 compensatedCollateral; // [WAD] amount of borrower collateral that is compensated with LPs (NFTs only)
         bool    inAuction;             // true if loan still in auction after repay, false otherwise
-        uint256 newLup;                // [WAD] LUP after repay debt action
         bool    pull;                  // true if pull action
         bool    repay;                 // true if repay action
         bool    stampT0Np;             // true if loan's t0 neutral price should be restamped (when repay settles auction or pull collateral)
@@ -71,7 +72,7 @@ library BorrowerActions {
     error BorrowerNotSender();
     error BorrowerUnderCollateralized();
     error InsufficientCollateral();
-    error LimitIndexReached();
+    error LimitIndexExceeded();
     error NoDebt();
 
     /***************************/
@@ -96,7 +97,7 @@ library BorrowerActions {
      *  @dev    reverts on:
      *              - borrower not sender BorrowerNotSender()
      *              - borrower debt less than pool min debt AmountLTMinDebt()
-     *              - limit price reached LimitIndexReached()
+     *              - limit price reached LimitIndexExceeded()
      *              - borrower cannot draw more debt BorrowerUnderCollateralized()
      *  @dev    emit events:
      *              - Auctions._settleAuction:
@@ -198,13 +199,13 @@ library BorrowerActions {
             result_.t0PoolDebt += vars.t0DebtChange;
             result_.poolDebt   = Maths.wmul(result_.t0PoolDebt, poolState_.inflator);
 
-            // 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 = _lup(deposits_, result_.poolDebt);
+
+            // revert if borrow drives LUP price under the specified price limit
+            _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 +255,7 @@ library BorrowerActions {
      *              - borrower debt less than pool min debt AmountLTMinDebt()
      *              - borrower not sender BorrowerNotSender()
      *              - not enough collateral to pull InsufficientCollateral()
+     *              - limit price reached LimitIndexExceeded()
      *  @dev    emit events:
      *              - Auctions._settleAuction:
      *                  - AuctionNFTSettle or AuctionSettle
@@ -266,7 +268,8 @@ library BorrowerActions {
         PoolState calldata poolState_,
         address borrowerAddress_,
         uint256 maxQuoteTokenAmountToRepay_,
-        uint256 collateralAmountToPull_
+        uint256 collateralAmountToPull_,
+        uint256 limitIndex_
     ) external returns (
         RepayDebtResult memory result_
     ) {
@@ -356,6 +359,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();
@@ -404,18 +409,11 @@ library BorrowerActions {
         return auctions_.liquidations[borrower_].kickTime != 0;
     }
 
-    function _lupIndex(
-        DepositsState storage deposits_,
-        uint256 debt_
-    ) internal view returns (uint256) {
-        return Deposits.findIndexOfSum(deposits_, debt_);
-    }
-
     function _lup(
         DepositsState storage deposits_,
         uint256 debt_
     ) internal view returns (uint256) {
-        return _priceAt(_lupIndex(deposits_, debt_));
+        return _priceAt(Deposits.findIndexOfSum(deposits_, debt_));
     }
 
 }