From ae92ac854724c0826bbba8a0530d912d0649c72d Mon Sep 17 00:00:00 2001 From: Hayden Shively <17186559+haydenshively@users.noreply.github.com> Date: Sat, 14 Oct 2023 19:55:47 -0500 Subject: [PATCH] Add a few more comments for audit --- core/src/Borrower.sol | 15 +++++++++--- core/src/Ledger.sol | 11 +++++++++ core/src/Lender.sol | 24 +++++++++++++++++- core/src/libraries/BalanceSheet.sol | 38 +++++++++++++++++++++++++++++ core/src/libraries/Oracle.sol | 2 +- 5 files changed, 85 insertions(+), 5 deletions(-) diff --git a/core/src/Borrower.sol b/core/src/Borrower.sol index 8d5f78c7..f1bb4ed8 100644 --- a/core/src/Borrower.sol +++ b/core/src/Borrower.sol @@ -150,7 +150,7 @@ contract Borrower is IUniswapV3MintCallback { * forced to call this in cases where the 5% swap bonus is up for grabs. * @param oracleSeed The indices of `UNISWAP_POOL.observations` where we start our search for * the 30-minute-old (lowest 16 bits) and 60-minute-old (next 16 bits) observations when getting - * TWAPs. If any of the highest 8 bits are set, we fallback to binary search. + * TWAPs. If any of the highest 8 bits are set, we fallback to onchain binary search. */ function warn(uint40 oracleSeed) external { uint256 slot0_ = slot0; @@ -189,7 +189,7 @@ contract Borrower is IUniswapV3MintCallback { * `3` one third, and so on. * @param oracleSeed The indices of `UNISWAP_POOL.observations` where we start our search for * the 30-minute-old (lowest 16 bits) and 60-minute-old (next 16 bits) observations when getting - * TWAPs. If any of the highest 8 bits are set, we fallback to binary search. + * TWAPs. If any of the highest 8 bits are set, we fallback to onchain binary search. */ function liquidate(ILiquidator callee, bytes calldata data, uint256 strain, uint40 oracleSeed) external { uint256 slot0_ = slot0; @@ -294,7 +294,7 @@ contract Borrower is IUniswapV3MintCallback { * @param data Encoded parameters that get forwarded to `callee` * @param oracleSeed The indices of `UNISWAP_POOL.observations` where we start our search for * the 30-minute-old (lowest 16 bits) and 60-minute-old (next 16 bits) observations when getting - * TWAPs. If any of the highest 8 bits are set, we fallback to binary search. + * TWAPs. If any of the highest 8 bits are set, we fallback to onchain binary search. */ function modify(IManager callee, bytes calldata data, uint40 oracleSeed) external payable { uint256 slot0_ = slot0; @@ -454,6 +454,15 @@ contract Borrower is IUniswapV3MintCallback { return extract(slot0); } + /** + * @notice Summarizes all oracle data pertinent to account health + * @dev If `seemsLegit == false`, you can call `Factory.pause` to temporarily disable borrows + * @param oracleSeed The indices of `UNISWAP_POOL.observations` where we start our search for + * the 30-minute-old (lowest 16 bits) and 60-minute-old (next 16 bits) observations when getting + * TWAPs. If any of the highest 8 bits are set, we fallback to onchain binary search. + * @return prices The probe prices currently being used to evaluate account health + * @return seemsLegit Whether the Uniswap TWAP seems to have been manipulated or not + */ function getPrices(uint40 oracleSeed) public view returns (Prices memory prices, bool seemsLegit) { (, uint8 nSigma, uint8 manipulationThresholdDivisor, ) = FACTORY.getParameters(UNISWAP_POOL); (prices, seemsLegit) = _getPrices(oracleSeed, nSigma, manipulationThresholdDivisor); diff --git a/core/src/Ledger.sol b/core/src/Ledger.sol index f60c62cd..d1b6cb49 100644 --- a/core/src/Ledger.sol +++ b/core/src/Ledger.sol @@ -209,6 +209,10 @@ contract Ledger { } } + /** + * @notice The amount of `asset` owed by `account` after accruing the latest interest. If one calls + * `repay(borrowBalance(account), account)`, the `account` will be left with a borrow balance of 0. + */ function borrowBalance(address account) external view returns (uint256) { uint256 b = borrows[account]; @@ -218,6 +222,7 @@ contract Ledger { } } + /// @notice The amount of `asset` owed by `account` before accruing the latest interest. function borrowBalanceStored(address account) external view returns (uint256) { uint256 b = borrows[account]; @@ -325,6 +330,10 @@ contract Ledger { HELPERS //////////////////////////////////////////////////////////////*/ + /** + * @dev Accrues interest up to the current `block.timestamp`. Updates and returns `cache`, but doesn't write + * anything to storage. + */ function _previewInterest(Cache memory cache) internal view returns (Cache memory, uint256, uint256) { unchecked { // Guard against reentrancy @@ -337,6 +346,7 @@ contract Ledger { return (cache, oldInventory, cache.totalSupply); } + // sload `reserveFactor` and `rateModel` at the same time since they're in the same slot uint8 rf = reserveFactor; uint256 accrualFactor = rateModel.getAccrualFactor({ utilization: (1e18 * oldBorrows) / oldInventory, @@ -376,6 +386,7 @@ contract Ledger { return roundUp ? shares.mulDivUp(inventory, totalSupply_) : shares.mulDivDown(inventory, totalSupply_); } + /// @dev The `account`'s balance, minus any shares earned by their courier function _nominalShares( address account, uint256 inventory, diff --git a/core/src/Lender.sol b/core/src/Lender.sol index 6f5a482e..8787a199 100644 --- a/core/src/Lender.sol +++ b/core/src/Lender.sol @@ -74,6 +74,7 @@ contract Lender is Ledger { Rewards.setRate(rate); } + /// @notice Allows `borrower` to call `borrow`. One the `FACTORY` can call this. function whitelist(address borrower) external { // Requirements: // - `msg.sender == FACTORY` so that only the factory can whitelist borrowers @@ -211,6 +212,7 @@ contract Lender is Ledger { BORROW/REPAY LOGIC //////////////////////////////////////////////////////////////*/ + /// @notice Sends `amount` of `asset` to `recipient` and increases `msg.sender`'s debt by `units` function borrow(uint256 amount, address recipient) external returns (uint256 units) { uint256 b = borrows[msg.sender]; require(b != 0, "Aloe: not a borrower"); @@ -238,6 +240,20 @@ contract Lender is Ledger { emit Borrow(msg.sender, recipient, amount, units); } + /** + * @notice Reduces `beneficiary`'s debt by `units`, assuming someone has pre-paid `amount` of `asset`. To repay + * all debt for some account, call `repay(borrowBalance(account), account)`. + * @dev To avoid frontrunning, `amount` should be pre-paid in the same transaction as the `repay` call. + * @custom:example ```solidity + * PERMIT2.permitTransferFrom( + * permitMsg, + * IPermit2.SignatureTransferDetails({to: address(lender), requestedAmount: amount}), + * msg.sender, + * signature + * ); + * lender.repay(amount, beneficiary) + * ``` + */ function repay(uint256 amount, address beneficiary) external returns (uint256 units) { uint256 b = borrows[beneficiary]; @@ -270,7 +286,12 @@ contract Lender is Ledger { emit Repay(msg.sender, beneficiary, amount, units); } - /// @dev Reentrancy guard is critical here! Without it, one could use a flash loan to repay a normal loan. + /** + * @notice Gives `to` temporary control over `amount` of `asset` in the `IFlashBorrower.onFlashLoan` callback. + * Arbitrary `data` can be forwarded to the callback. Before returning, the `IFlashBorrower` must have sent + * at least `amount` back to this contract. + * @dev Reentrancy guard is critical here! Without it, one could use a flash loan to repay a normal loan. + */ function flash(uint256 amount, IFlashBorrower to, bytes calldata data) external { // Guard against reentrancy uint32 lastAccrualTime_ = lastAccrualTime; @@ -374,6 +395,7 @@ contract Lender is Ledger { HELPERS //////////////////////////////////////////////////////////////*/ + /// @dev Transfers `shares` from `from` to `to`, iff neither of them have a courier function _transfer(address from, address to, uint256 shares) private { (Rewards.Storage storage s, uint144 a) = Rewards.load(); diff --git a/core/src/libraries/BalanceSheet.sol b/core/src/libraries/BalanceSheet.sol index 9902d8fc..0dc843ca 100644 --- a/core/src/libraries/BalanceSheet.sol +++ b/core/src/libraries/BalanceSheet.sol @@ -15,17 +15,26 @@ import {square, mulDiv128, mulDiv128Up} from "./MulDiv.sol"; import {TickMath} from "./TickMath.sol"; struct Assets { + // The `Borrower`'s balance of `TOKEN0`, i.e. `TOKEN0.balanceOf(borrower)` uint256 fixed0; + // The `Borrower`'s balance of `TOKEN1`, i.e. `TOKEN1.balanceOf(borrower)` uint256 fixed1; + // The value of the `Borrower`'s Uniswap liquidity, evaluated at `Prices.a`, denominated in `TOKEN1` uint256 fluid1A; + // The value of the `Borrower`'s Uniswap liquidity, evaluated at `Prices.b`, denominated in `TOKEN1` uint256 fluid1B; + // The amount of `TOKEN0` underlying the `Borrower`'s Uniswap liquidity, evaluated at `Prices.c` uint256 fluid0C; + // The amount of `TOKEN1` underlying the `Borrower`'s Uniswap liquidity, evaluated at `Prices.c` uint256 fluid1C; } struct Prices { + // Some sqrtPriceX96 *less* than the current TWAP uint160 a; + // Some sqrtPriceX96 *greater* than the current TWAP uint160 b; + // The current TWAP, expressed as a sqrtPriceX96 uint160 c; } @@ -35,6 +44,7 @@ struct Prices { library BalanceSheet { using SoladyMath for uint256; + /// @dev Checks whether a `Borrower` is healthy given the probe prices and its current assets and liabilities function isHealthy( Prices memory prices, Assets memory mem, @@ -69,6 +79,18 @@ library BalanceSheet { return true; } + /** + * Given data from the `ORACLE` (first 3 args) and parameters from the `FACTORY` (last 2 args), computes + * the probe prices at which to check the account's health + * @param metric The manipulation metric (from oracle) + * @param sqrtMeanPriceX96 The current TWAP, expressed as a sqrtPriceX96 (from oracle) + * @param iv The estimated implied volatility, expressed as a 1e12 percentage (from oracle) + * @param nSigma The number of standard deviations of price movement to account for (from factory) + * @param manipulationThresholdDivisor Helps compute the manipulation threshold (from factory). See `Constants.sol` + * @return a \\( \text{TWAP} \cdot e^{-n \cdot \sigma} \\) expressed as a sqrtPriceX96 + * @return b \\( \text{TWAP} \cdot e^{+n \cdot \sigma} \\) expressed as a sqrtPriceX96 + * @return seemsLegit Whether the Uniswap TWAP has been manipulated enough to create bad debt at the effective LTV + */ function computeProbePrices( uint56 metric, uint256 sqrtMeanPriceX96, @@ -77,6 +99,7 @@ library BalanceSheet { uint8 manipulationThresholdDivisor ) internal pure returns (uint160 a, uint160 b, bool seemsLegit) { unchecked { + // Essentially sqrt(e^{nSigma*iv}). Note the `Factory` defines `nSigma` with an extra factor of 10 uint256 sqrtScaler = uint256(exp1e12(int256((nSigma * iv) / 20))).clamp( PROBE_SQRT_SCALER_MIN, PROBE_SQRT_SCALER_MAX @@ -89,6 +112,16 @@ library BalanceSheet { } } + /** + * @notice Computes the liquidation incentive that would be paid out if a liquidator closes the account + * using a swap with `strain = 1` + * @param assets0 The amount of `TOKEN0` held/controlled by the `Borrower` at the current TWAP + * @param assets1 The amount of `TOKEN1` held/controlled by the `Borrower` at the current TWAP + * @param liabilities0 The amount of `TOKEN0` that the `Borrower` owes to `LENDER0` + * @param liabilities1 The amount of `TOKEN1` that the `Borrower` owes to `LENDER1` + * @param meanPriceX128 The current TWAP + * @return incentive1 The incentive to pay out, denominated in `TOKEN1` + */ function computeLiquidationIncentive( uint256 assets0, uint256 assets1, @@ -122,6 +155,11 @@ library BalanceSheet { } } + /** + * @notice The effective LTV implied by `sqrtScaler`. This LTV is accurate for fixed assets and out-of-range + * Uniswap positions, but not for in-range Uniswap positions (impermanent losses make their effective LTV + * slightly smaller). + */ function _ltv(uint256 sqrtScaler) private pure returns (uint160 ltv) { unchecked { ltv = uint160(LTV_NUMERATOR.rawDiv(sqrtScaler * sqrtScaler)); diff --git a/core/src/libraries/Oracle.sol b/core/src/libraries/Oracle.sol index 76796ce2..fa8eb93e 100644 --- a/core/src/libraries/Oracle.sol +++ b/core/src/libraries/Oracle.sol @@ -33,7 +33,7 @@ library Oracle { * @param pool Address of the pool that we want to observe * @param seed The indices of `pool.observations` where we start our search for the 30-minute-old (lowest 16 bits) * and 60-minute-old (next 16 bits) observations. Determine these off-chain to make this method more efficient - * than Uniswap's binary search. If any of the highest 8 bits are set, we fallback to binary search. + * than Uniswap's binary search. If any of the highest 8 bits are set, we fallback to onchain binary search. * @return data An up-to-date `PoolData` struct containing all fields except `oracleLookback` and `tickLiquidity` * @return metric If the price was manipulated at any point in the past `UNISWAP_AVG_WINDOW` seconds, then at * some point in that period, this value will spike. It may still be high now, or (if the attacker is smart and