Merge branch 'develop' into fix-reserves-draining

.github/pull_request_template.md

@@ -1,37 +1,24 @@
+## Description
 
-<!---
-No need to add special tag
-src/ & non src/ changes you need the following (that apply):
--->
-# Description of change
-## High level
-* <DESCRIP_OF_CHANGE>
-  * <DESCRIP_OF_SUBCHANGE>
+<!-- Explain what was changed.  For example:
+_Updated rounding in `removeQuoteToken` to round to token precision._ -->
 
-<!---
-Add the `Status: Needs Auditor Approval` tags
-CHANGES IN /SRC DIR:
-- renaming (not retyping or resizing) of variables & methods
-- reordering and moving of functions in files
-- lite moving of functions accross files
-- comments
+## Purpose
 
-src/ changes you need the following (that apply):
--->
+<!-- Explain why the change was made, citing any issues where appropriate.  For example:
+_Resolves audit issue M-333: Removal of quote token may leave dust amounts._
+Or, if the change does not affect deployed contracts: _Resolve rounding issue with invariant E9 to handle tokens with less than 8 decimals._ -->
 
-# Description of bug or vulnerability and solution
-* <PARAGRAPH_EXP_OF_VULN_BUG>
-* <PARAGRAPH_EXP_OF_HOW_CHANGE_SOLVES_VULN_OR_BUG>
+## Impact
 
-# Contract size
-## Pre Change
-<PASTE_OUTPUT_HERE>
-## Post Change
-<PASTE_OUTPUT_HERE>
+<!-- State technical consequences of the change, whether beneficial or detrimental.  For example:
+_Small increase in `removeQuoteToken` gas cost._
+If the change does not affect deployed contracts, feel free to leave _none_. -->
 
-# Gas usage
-## Pre Change
-<PASTE_OUTPUT_HERE>
-## Post Change
-<PASTE_OUTPUT_HERE>
+## Tasks
 
+- [ ] Changes to protocol contracts are covered by unit tests executed by CI.
+- [ ] Protocol contract size limits have not been exceeded.
+- [ ] Gas consumption for impacted transactions have been compared with the target branch, and nontrivial changes cited in the _Impact_ section above.
+- [ ] Scope labels have been assigned as appropriate.
+- [ ] Invariant tests have been manually executed as appropriate for the nature of the change.

src/ERC20Pool.sol

@@ -345,7 +345,7 @@ contract ERC20Pool is FlashloanablePool, IERC20Pool {
     function settle(
         address borrowerAddress_,
         uint256 maxDepth_
-    ) external override nonReentrant {
+    ) external override nonReentrant returns (uint256 collateralSettled_, bool isBorrowerSettled_) {
         PoolState memory poolState = _accruePoolInterest();
 
         SettleResult memory result = SettlerActions.settlePoolDebt(
@@ -363,6 +363,9 @@ contract ERC20Pool is FlashloanablePool, IERC20Pool {
         );
 
         _updatePostSettleState(result, poolState);
+
+        collateralSettled_ = result.collateralSettled;
+        isBorrowerSettled_ = (result.debtPostAction == 0);
     }
 
     /**

src/ERC721Pool.sol

@@ -344,7 +344,6 @@ contract ERC721Pool is FlashloanablePool, IERC721Pool {
             // Total collateral in buckets meets the requested removal amount, noOfNFTsToRemove_
             _transferFromPoolToAddress(msg.sender, bucketTokenIds, noOfNFTsToRemove_);
         }
-
     }
 
     /**
@@ -394,7 +393,7 @@ contract ERC721Pool is FlashloanablePool, IERC721Pool {
     function settle(
         address borrowerAddress_,
         uint256 maxDepth_
-    ) external nonReentrant override {
+    ) external nonReentrant override returns (uint256 collateralSettled_, bool isBorrowerSettled_) {
         PoolState memory poolState = _accruePoolInterest();
 
         SettleParams memory params = SettleParams({
@@ -417,6 +416,9 @@ contract ERC721Pool is FlashloanablePool, IERC721Pool {
 
         // move token ids from borrower array to pool claimable array if any collateral used to settle bad debt
         _rebalanceTokens(params.borrower, result.collateralRemaining);
+
+        collateralSettled_ = result.collateralSettled;
+        isBorrowerSettled_ = (result.debtPostAction == 0);
     }
 
     /**

src/PoolInfoUtils.sol

@@ -454,7 +454,6 @@ contract PoolInfoUtils {
             bucketCollateral,
             bucketDeposit,
             lp_,
-            bucketDeposit,
             _priceAt(index_)
         );
     }

src/PositionManager.sol

@@ -5,6 +5,7 @@ pragma solidity 0.8.18;
 import { ERC20 }           from '@openzeppelin/contracts/token/ERC20/ERC20.sol';
 import { IERC20 }          from '@openzeppelin/contracts/token/ERC20/IERC20.sol';
 import { EnumerableSet }   from '@openzeppelin/contracts/utils/structs/EnumerableSet.sol';
+import { Multicall }       from '@openzeppelin/contracts/utils/Multicall.sol';
 import { ReentrancyGuard } from '@openzeppelin/contracts/security/ReentrancyGuard.sol';
 import { SafeERC20 }       from '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol';
 
@@ -36,7 +37,7 @@ import { PositionNFTSVG } from './libraries/external/PositionNFTSVG.sol';
  *          - `redeem` positions for given buckets
  *          - `burn` positions `NFT`
  */
-contract PositionManager is PermitERC721, IPositionManager, ReentrancyGuard {
+contract PositionManager is PermitERC721, IPositionManager, Multicall, ReentrancyGuard {
     using EnumerableSet for EnumerableSet.UintSet;
     using SafeERC20     for ERC20;
 
@@ -324,7 +325,6 @@ contract PositionManager is PermitERC721, IPositionManager, ReentrancyGuard {
             vars.bucketCollateral,
             vars.bucketDeposit,
             vars.fromLP,
-            vars.bucketDeposit,
             _priceAt(fromIndex_)
         );
 

src/base/FlashloanablePool.sol

@@ -6,6 +6,7 @@ import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.s
 import { IERC20 }    from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 
 import { Pool }                  from './Pool.sol';
+import { PoolCommons }           from '../libraries/external/PoolCommons.sol';
 import { IERC3156FlashBorrower } from '../interfaces/pool/IERC3156FlashBorrower.sol';
 
 /**
@@ -32,30 +33,7 @@ abstract contract FlashloanablePool is Pool {
         bytes calldata data_
     ) external virtual override nonReentrant returns (bool success_) {
         if (!_isFlashloanSupported(token_)) revert FlashloanUnavailableForToken();
-
-        IERC20 tokenContract = IERC20(token_);
-
-        uint256 initialBalance = tokenContract.balanceOf(address(this));
-
-        tokenContract.safeTransfer(
-            address(receiver_),
-            amount_
-        );
-
-        if (receiver_.onFlashLoan(msg.sender, token_, amount_, 0, data_) != 
-            keccak256("ERC3156FlashBorrower.onFlashLoan")) revert FlashloanCallbackFailed();
-
-        tokenContract.safeTransferFrom(
-            address(receiver_),
-            address(this),
-            amount_
-        );
-
-        if (tokenContract.balanceOf(address(this)) != initialBalance) revert FlashloanIncorrectBalance();
-
-        success_ = true;
-
-        emit Flashloan(address(receiver_), token_, amount_);
+        success_ = PoolCommons.flashLoan(receiver_, token_, amount_, data_);
     }
 
     /**

src/base/Pool.sol

@@ -50,6 +50,7 @@ import {
 }                                   from '../interfaces/pool/commons/IPoolInternals.sol';
 
 import {
+    _determineInflatorState,
     _priceAt,
     _roundToScale
 }                               from '../libraries/helpers/PoolHelper.sol';
@@ -360,7 +361,7 @@ abstract contract Pool is Clone, ReentrancyGuard, Multicall, IPool {
         poolBalances.t0DebtInAuction = poolState.t0DebtInAuction;
 
         // update pool interest rate state
-        _updateInterestState(poolState, result.lup);
+        _updateInterestState(poolState, Deposits.getLup(deposits, poolState.debt));
 
         // transfer from kicker to pool the difference to cover bond
         if (result.amountToCoverBond != 0) _transferQuoteTokenFrom(msg.sender, result.amountToCoverBond);
@@ -405,10 +406,13 @@ abstract contract Pool is Clone, ReentrancyGuard, Multicall, IPool {
      *  @dev    update `reserveAuction.latestBurnEventEpoch` and burn event `timestamp` state
      *  @dev    === Reverts on ===
      *  @dev    2 weeks not passed `ReserveAuctionTooSoon()`
+     *  @dev    unsettled liquidation `AuctionNotCleared()`
      *  @dev    === Emit events ===
      *  @dev    - `KickReserveAuction`
      */
     function kickReserveAuction() external override nonReentrant {
+        _revertIfAuctionClearable(auctions, loans);
+
         // start a new claimable reserve auction, passing in relevant parameters such as the current pool size, debt, balance, and inflator value
         KickerActions.kickReserveAuction(
             auctions,
@@ -561,19 +565,23 @@ abstract contract Pool is Clone, ReentrancyGuard, Multicall, IPool {
 
             // if new interest may have accrued, call accrueInterest function and update inflator and debt fields of poolState_ struct
             if (poolState_.isNewInterestAccrued) {
-                (uint256 newInflator, uint256 newInterest) = PoolCommons.accrueInterest(
+                try PoolCommons.accrueInterest(
                     emaState,
                     deposits,
                     poolState_,
                     Loans.getMax(loans).thresholdPrice,
                     elapsed
-                );
-                poolState_.inflator = newInflator;
-                // After debt owed to lenders has accrued, calculate current debt owed by borrowers
-                poolState_.debt = Maths.wmul(poolState_.t0Debt, poolState_.inflator);
-
-                // update total interest earned accumulator with the newly accrued interest
-                reserveAuction.totalInterestEarned += newInterest;
+                ) returns (uint256 newInflator, uint256 newInterest) {
+                    poolState_.inflator = newInflator;
+                    // After debt owed to lenders has accrued, calculate current debt owed by borrowers
+                    poolState_.debt = Maths.wmul(poolState_.t0Debt, poolState_.inflator);
+
+                    // update total interest earned accumulator with the newly accrued interest
+                    reserveAuction.totalInterestEarned += newInterest;
+                } catch {
+                    poolState_.isNewInterestAccrued = false;
+                    emit InterestUpdateFailure();
+                }
             }
         }
     }
@@ -679,19 +687,13 @@ abstract contract Pool is Clone, ReentrancyGuard, Multicall, IPool {
         PoolState memory poolState_,
         uint256 lup_
     ) internal {
-
-        PoolCommons.updateInterestState(interestState, emaState, deposits, poolState_, lup_);
-
-        // update pool inflator
-        if (poolState_.isNewInterestAccrued) {
-            inflatorState.inflator       = uint208(poolState_.inflator);
-            inflatorState.inflatorUpdate = uint48(block.timestamp);
-        // if the debt in the current pool state is 0, also update the inflator and inflatorUpdate fields in inflatorState
-        // slither-disable-next-line incorrect-equality
-        } else if (poolState_.debt == 0) {
-            inflatorState.inflator       = uint208(Maths.WAD);
-            inflatorState.inflatorUpdate = uint48(block.timestamp);
+        try PoolCommons.updateInterestState(interestState, emaState, deposits, poolState_, lup_) {} catch {
+            emit InterestUpdateFailure();
         }
+
+        (uint208 newInflator, bool updateTimestamp) = _determineInflatorState(poolState_, inflatorState);
+        inflatorState.inflator = newInflator;
+        if (updateTimestamp) inflatorState.inflatorUpdate = uint48(block.timestamp);
     }
 
     /**

src/base/PoolDeployer.sol

@@ -77,6 +77,7 @@ abstract contract PoolDeployer {
      * @notice Returns the list of all deployed pools.
      * @dev    This function is used by integrations to access deployed pools.
      * @dev    Each factory implementation maintains its own list of deployed pools.
+     * @dev    This method should only be used by off-chain integrations.
      * @return List of all deployed pools.
      */
     function getDeployedPoolsList() external view returns (address[] memory) {

src/interfaces/pool/commons/IPoolEvents.sol

@@ -367,4 +367,9 @@ interface IPoolEvents {
         uint256 newRate
     );
 
+    /**
+     *  @notice Emitted when interest accural or update interest overflows.
+     */
+    event InterestUpdateFailure();
+
 }

src/interfaces/pool/commons/IPoolSettlerActions.sol

@@ -9,13 +9,15 @@ interface IPoolSettlerActions {
 
     /**
      *  @notice Called by actors to settle an amount of debt in a completed liquidation.
-     *  @param  borrowerAddress_ Address of the auctioned borrower.
-     *  @param  maxDepth_        Measured from `HPB`, maximum number of buckets deep to settle debt.
+     *  @param  borrowerAddress_   Address of the auctioned borrower.
+     *  @param  maxDepth_          Measured from `HPB`, maximum number of buckets deep to settle debt.
+     *  @return collateralSettled_ Amount of collateral settled.
+     *  @return isBorrowerSettled_ Is all borrower's debt is settled.
      *  @dev    `maxDepth_` is used to prevent unbounded iteration clearing large liquidations.
      */
     function settle(
         address borrowerAddress_,
         uint256 maxDepth_
-    ) external;
+    ) external returns (uint256 collateralSettled_, bool isBorrowerSettled_);
 
 }

src/libraries/external/KickerActions.sol

@@ -27,7 +27,6 @@ import {
 import {
     MAX_INFLATED_PRICE,
     _bondParams,
-    _bpf,
     _claimableReserves,
     _isCollateralized,
     _priceAt,
@@ -204,11 +203,10 @@ library KickerActions {
         KickReserveAuctionParams calldata params_
     ) external {
         // retrieve timestamp of latest burn event and last burn timestamp
-        uint256 latestBurnEpoch   = reserveAuction_.latestBurnEventEpoch;
-        uint256 lastBurnTimestamp = reserveAuction_.burnEvents[latestBurnEpoch].timestamp;
+        uint256 latestBurnEpoch = reserveAuction_.latestBurnEventEpoch;
 
-        // check that at least two weeks have passed since the last reserve auction completed, and that the auction was not kicked within the past 72 hours
-        if (block.timestamp < lastBurnTimestamp + 2 weeks || block.timestamp - reserveAuction_.kicked <= 72 hours) {
+        // check that at least two weeks have passed since the last reserve auction completed
+        if (block.timestamp < reserveAuction_.kicked + 2 weeks + 72 hours) {
             revert ReserveAuctionTooSoon();
         }