diff --git a/core/src/Lender.sol b/core/src/Lender.sol index effdaf47..0b58be51 100644 --- a/core/src/Lender.sol +++ b/core/src/Lender.sol @@ -61,7 +61,7 @@ contract Lender is Ledger { /// @notice Sets the `rateModel` and `reserveFactor`. Only the `FACTORY` can call this. function setRateModelAndReserveFactor(IRateModel rateModel_, uint8 reserveFactor_) external { - require(msg.sender == address(FACTORY)); + require(msg.sender == address(FACTORY) && reserveFactor_ > 0); rateModel = rateModel_; reserveFactor = reserveFactor_; } diff --git a/core/src/VolatilityOracle.sol b/core/src/VolatilityOracle.sol index 96356eb5..6cf0c5f3 100644 --- a/core/src/VolatilityOracle.sol +++ b/core/src/VolatilityOracle.sol @@ -6,7 +6,7 @@ import {IUniswapV3Pool} from "v3-core/contracts/interfaces/IUniswapV3Pool.sol"; import { IV_SCALE, IV_COLD_START, - IV_CHANGE_PER_SECOND, + IV_CHANGE_PER_UPDATE, UNISWAP_AVG_WINDOW, FEE_GROWTH_AVG_WINDOW, FEE_GROWTH_ARRAY_LENGTH, @@ -53,8 +53,7 @@ contract VolatilityOracle { // If fewer than `FEE_GROWTH_SAMPLE_PERIOD` seconds have elapsed, return early. // We still fetch the latest TWAP, but we do not sample feeGrowthGlobals or update IV. - uint256 timeSinceLastWrite = block.timestamp - lastWrite.time; - if (timeSinceLastWrite < FEE_GROWTH_SAMPLE_PERIOD) { + if (block.timestamp - lastWrite.time < FEE_GROWTH_SAMPLE_PERIOD) { return (metric, data.sqrtMeanPriceX96, lastWrite.iv); } @@ -80,9 +79,8 @@ contract VolatilityOracle { // Estimate, then clamp so it lies within [previous - maxChange, previous + maxChange] iv = Volatility.estimate(cachedMetadata[pool], data, a, b, IV_SCALE); - uint256 maxChange = timeSinceLastWrite * IV_CHANGE_PER_SECOND; - if (iv > lastWrite.iv + maxChange) iv = lastWrite.iv + maxChange; - else if (iv + maxChange < lastWrite.iv) iv = lastWrite.iv - maxChange; + if (iv > lastWrite.iv + IV_CHANGE_PER_UPDATE) iv = lastWrite.iv + IV_CHANGE_PER_UPDATE; + else if (iv + IV_CHANGE_PER_UPDATE < lastWrite.iv) iv = lastWrite.iv - IV_CHANGE_PER_UPDATE; } // Store the new feeGrowthGlobals sample and update `lastWrites` diff --git a/core/src/libraries/Rewards.sol b/core/src/libraries/Rewards.sol index d33dfcb0..2bd1278f 100644 --- a/core/src/libraries/Rewards.sol +++ b/core/src/libraries/Rewards.sol @@ -10,7 +10,7 @@ import {log2Up, exp2} from "./Log2.sol"; library Rewards { event RewardsRateSet(uint56 rate); - event RewardsClaimed(address user, uint112 amount); + event RewardsClaimed(address indexed user, uint112 amount); bytes32 private constant _REWARDS_SLOT = keccak256("aloe.ii.rewards"); diff --git a/core/src/libraries/constants/Constants.sol b/core/src/libraries/constants/Constants.sol index f2ce89c5..14376c16 100644 --- a/core/src/libraries/constants/Constants.sol +++ b/core/src/libraries/constants/Constants.sol @@ -134,6 +134,11 @@ uint128 constant IV_COLD_START = 0.30e12; /// change by 0.0000463 percentage points per second → 4 percentage points per day. uint256 constant IV_CHANGE_PER_SECOND = 462962; +/// @dev The maximum amount by which (reported) implied volatility can change with a single `VolatilityOracle.update` +/// call. If updates happen as frequently as possible (every `FEE_GROWTH_SAMPLE_PERIOD`), this cap is no different +/// from `IV_CHANGE_PER_SECOND` alone. +uint256 constant IV_CHANGE_PER_UPDATE = IV_CHANGE_PER_SECOND * FEE_GROWTH_SAMPLE_PERIOD; + /// @dev To estimate volume, we need 2 samples. One is always at the current block, the other is from /// `FEE_GROWTH_AVG_WINDOW` seconds ago, +/- `FEE_GROWTH_SAMPLE_PERIOD / 2`. Larger values make the resulting volume /// estimate more robust, but may cause the oracle to miss brief spikes in activity. @@ -142,11 +147,11 @@ uint256 constant FEE_GROWTH_AVG_WINDOW = 6 hours; /// @dev The length of the circular buffer that stores feeGrowthGlobals samples. /// Must be in interval /// \\( \left[ \frac{\text{FEE_GROWTH_AVG_WINDOW}}{\text{FEE_GROWTH_SAMPLE_PERIOD}}, 256 \right) \\) -uint256 constant FEE_GROWTH_ARRAY_LENGTH = 48; +uint256 constant FEE_GROWTH_ARRAY_LENGTH = 32; /// @dev The minimum number of seconds that must elapse before a new feeGrowthGlobals sample will be stored. This /// controls how often the oracle can update IV. -uint256 constant FEE_GROWTH_SAMPLE_PERIOD = 15 minutes; +uint256 constant FEE_GROWTH_SAMPLE_PERIOD = 1 hours; /// @dev To compute Uniswap mean price & liquidity, we need 2 samples. One is always at the current block, the other is /// from `UNISWAP_AVG_WINDOW` seconds ago. Larger values make the resulting price/liquidity values harder to diff --git a/core/test/LenderRewards.t.sol b/core/test/LenderRewards.t.sol new file mode 100644 index 00000000..8c8bd74c --- /dev/null +++ b/core/test/LenderRewards.t.sol @@ -0,0 +1,249 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity 0.8.17; + +import "forge-std/Test.sol"; + +import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; + +import "src/Lender.sol"; +import "src/RateModel.sol"; + +import {FactoryForLenderTests} from "./Utils.sol"; + +contract LenderRewardsTest is Test { + uint256 private constant REWARDS_RATE_MIN = uint256(1e19) / (365 days); + uint256 private constant REWARDS_RATE_MAX = uint256(1e24) / (365 days); + + event RewardsRateSet(uint56 rate); + + event RewardsClaimed(address indexed user, uint112 amount); + + MockERC20 rewardsToken; + + MockERC20 asset; + + FactoryForLenderTests factory; + + Lender lender; + + function setUp() public { + rewardsToken = new MockERC20("Rewards Token", "RWD", 18); + asset = new MockERC20("Test Token", "TKN", 18); + + factory = new FactoryForLenderTests(new RateModel(), rewardsToken); + lender = factory.deploySingleLender(asset); + } + + function test_setRate(uint56 rate, address caller) public { + vm.assume(caller != factory.GOVERNOR()); + + // Starts at 0 + assertEq(lender.rewardsRate(), 0); + + // Only governance can change it + vm.prank(caller); + vm.expectRevert(bytes("")); + factory.governRewardsRate(lender, rate); + + // Set it to `rate` + vm.prank(factory.GOVERNOR()); + vm.expectEmit(true, false, false, false, address(lender)); + emit RewardsRateSet(rate); + factory.governRewardsRate(lender, rate); + assertEq(lender.rewardsRate(), rate); + } + + function test_nothingToClaimAtFirst(address owner, address caller) public { + vm.assume(caller != address(factory)); + + assertEq(lender.rewardsOf(owner), 0); + + // Only factory can claim + vm.prank(caller); + vm.expectRevert(bytes("")); + lender.claimRewards(owner); + + vm.prank(address(factory)); + vm.expectEmit(true, false, false, true, address(lender)); + emit RewardsClaimed(owner, 0); + uint256 earned = lender.claimRewards(owner); + assertEq(earned, 0); + } + + function test_accounting1Holder(address holder, uint56 rate0, uint56 rate1) public { + if (rate0 > 0) rate0 = uint56(bound(rate0, REWARDS_RATE_MIN, REWARDS_RATE_MAX)); + if (rate1 > 0) rate1 = uint56(bound(rate1, REWARDS_RATE_MIN, REWARDS_RATE_MAX)); + + // Set `rate0` + vm.prank(factory.GOVERNOR()); + factory.governRewardsRate(lender, rate0); + + // Rewards should begin accruing after deposit + asset.mint(address(lender), 1e18); + lender.deposit(1e18, holder); + + skip(1 days); + assertApproxEqRel(lender.rewardsOf(holder), uint256(rate0) * (1 days), 0.001e18); + + // Set `rate1` + vm.prank(factory.GOVERNOR()); + factory.governRewardsRate(lender, rate1); + + skip(1 days); + assertApproxEqRel(lender.rewardsOf(holder), (uint256(rate0) + rate1) * (1 days), 0.001e18); + + // Rewards should stop accruing after redeem + vm.prank(holder); + lender.redeem(type(uint256).max, holder, holder); + + skip(1 days); + assertApproxEqRel(lender.rewardsOf(holder), (uint256(rate0) + rate1) * (1 days), 0.001e18); + + // Check proper claim + uint112 earned = lender.rewardsOf(holder); + vm.prank(address(factory)); + vm.expectEmit(true, false, false, true, address(lender)); + emit RewardsClaimed(holder, earned); + assertEq(lender.claimRewards(holder), earned); + // Check no duplicate claim + assertEq(lender.rewardsOf(holder), 0); + vm.prank(address(factory)); + assertEq(lender.claimRewards(holder), 0); + } + + function test_accounting2Holders(address holder0, address holder1, uint56 rate) public { + vm.assume(holder0 != holder1); + if (rate > 0) rate = uint56(bound(rate, REWARDS_RATE_MIN, REWARDS_RATE_MAX)); + + // Set `rate` + vm.prank(factory.GOVERNOR()); + factory.governRewardsRate(lender, rate); + + // Rewards should begin accruing to holder0 after deposit + asset.mint(address(lender), 1e18); + lender.deposit(1e18, holder0); + + skip(1 days); + assertApproxEqRel(lender.rewardsOf(holder0), uint256(rate) * (1 days), 0.001e18); + assertEq(lender.rewardsOf(holder1), 0); + + // Send half the tokens to holder1 + vm.prank(holder0); + lender.transfer(holder1, 0.5e18); + + skip(1 days); + assertApproxEqRel(lender.rewardsOf(holder0), (uint256(rate) + rate / 2) * (1 days), 0.001e18); + assertApproxEqRel(lender.rewardsOf(holder1), uint256(rate / 2) * (1 days), 0.001e18); + + // Rewards should stop accruing to holder0 after redeem + vm.prank(holder0); + lender.redeem(type(uint256).max, holder0, holder0); + + skip(1 days); + assertApproxEqRel(lender.rewardsOf(holder0), (uint256(rate) + rate / 2) * (1 days), 0.001e18); + assertApproxEqRel(lender.rewardsOf(holder1), (uint256(rate) + rate / 2) * (1 days), 0.001e18); + + // Check proper claim for holder0 + uint112 earned = lender.rewardsOf(holder0); + vm.prank(address(factory)); + vm.expectEmit(true, false, false, true, address(lender)); + emit RewardsClaimed(holder0, earned); + assertEq(lender.claimRewards(holder0), earned); + + // Check proper claim for holder1 + earned = lender.rewardsOf(holder1); + vm.prank(address(factory)); + vm.expectEmit(true, false, false, true, address(lender)); + emit RewardsClaimed(holder1, earned); + assertEq(lender.claimRewards(holder1), earned); + } + + function test_selfTransfer(address holder, uint56 rate) public { + if (rate > 0) rate = uint56(bound(rate, REWARDS_RATE_MIN, REWARDS_RATE_MAX)); + + // Set `rate` + vm.prank(factory.GOVERNOR()); + factory.governRewardsRate(lender, rate); + + // Rewards should begin accruing to holder after deposit + asset.mint(address(lender), 1e18); + lender.deposit(1e18, holder); + + skip(1 days); + assertApproxEqRel(lender.rewardsOf(holder), uint256(rate) * (1 days), 0.001e18); + + // Send half the tokens to holder + vm.prank(holder); + lender.transfer(holder, 0.5e18); + + skip(1 days); + assertApproxEqRel(lender.rewardsOf(holder), uint256(rate) * (2 days), 0.001e18); + + // Rewards should stop accruing to holder after redeem + vm.prank(holder); + lender.redeem(type(uint256).max, holder, holder); + + skip(1 days); + assertApproxEqRel(lender.rewardsOf(holder), uint256(rate) * (2 days), 0.001e18); + + // Check proper claim for holder + uint112 earned = lender.rewardsOf(holder); + vm.prank(address(factory)); + vm.expectEmit(true, false, false, true, address(lender)); + emit RewardsClaimed(holder, earned); + assertEq(lender.claimRewards(holder), earned); + } + + function test_accountingBehavesAtExtremes(address holder0, address holder1, uint56 rate) public { + vm.assume(holder0 != holder1); + + // Set `rate` + vm.prank(factory.GOVERNOR()); + factory.governRewardsRate(lender, rate); + + // Max absolute error is 2, so we do this for assertLe's to pass + if (rate < type(uint56).max - 2) rate += 2; + + // Rewards should begin accruing to holder0 after deposit + asset.mint(address(lender), 1000000e18); + lender.deposit(1000000e18, holder0); + + console2.log(lender.balanceOf(holder0), lender.balanceOf(holder1), lender.totalSupply()); + + skip(365 days); + assertLe(lender.rewardsOf(holder0), uint256(rate) * (365 days), "excessive A0"); + assertEq(lender.rewardsOf(holder1), 0); + + // Send half the tokens to holder1 + vm.prank(holder0); + lender.transfer(holder1, 500000e18); + + console2.log(lender.balanceOf(holder0), lender.balanceOf(holder1), lender.totalSupply()); + + skip(365 days); + assertLe(lender.rewardsOf(holder0), uint256(rate) * (547.5 days), "excessive B0"); + assertLe(lender.rewardsOf(holder1), uint256(rate) * (182.5 days), "excessive B1"); + + // Rewards should stop accruing to holder0 after redeem + vm.prank(holder0); + lender.redeem(type(uint256).max, holder0, holder0); + + skip(365 days); + assertLe(lender.rewardsOf(holder0), uint256(rate) * (547.5 days), "excessive C0"); + assertLe(lender.rewardsOf(holder1), uint256(rate) * (547.5 days), "excessive C1"); + + // Check proper claim for holder0 + uint112 earned = lender.rewardsOf(holder0); + vm.prank(address(factory)); + vm.expectEmit(true, false, false, true, address(lender)); + emit RewardsClaimed(holder0, earned); + assertEq(lender.claimRewards(holder0), earned); + + // Check proper claim for holder1 + earned = lender.rewardsOf(holder1); + vm.prank(address(factory)); + vm.expectEmit(true, false, false, true, address(lender)); + emit RewardsClaimed(holder1, earned); + assertEq(lender.claimRewards(holder1), earned); + } +} diff --git a/core/test/Utils.sol b/core/test/Utils.sol index 7b15b945..d889a662 100644 --- a/core/test/Utils.sol +++ b/core/test/Utils.sol @@ -24,6 +24,7 @@ contract FactoryForLenderTests is Factory { function deploySingleLender(ERC20 asset) external returns (Lender) { address proxy = ClonesWithImmutableArgs.clone(LENDER_IMPLEMENTATION, abi.encodePacked(address(asset))); + isLender[proxy] = true; Lender(proxy).initialize(); Lender(proxy).setRateModelAndReserveFactor(DEFAULT_RATE_MODEL, 8); diff --git a/core/test/VolatilityOracle.t.sol b/core/test/VolatilityOracle.t.sol index c98f1be1..b04cb8a7 100644 --- a/core/test/VolatilityOracle.t.sol +++ b/core/test/VolatilityOracle.t.sol @@ -183,7 +183,7 @@ contract VolatilityOracleTest is Test { assertEqDecimal(ivStored, ivWritten, 12); assertEq(newIndex, (currentIndex + 1) % FEE_GROWTH_ARRAY_LENGTH); - uint256 maxChange = (newTime - currentTime) * IV_CHANGE_PER_SECOND; + uint256 maxChange = IV_CHANGE_PER_UPDATE; assertLe(ivWritten, currentIV + maxChange); assertGe(ivWritten + maxChange, currentIV); diff --git a/core/test/invariants/LenderInvariants.t.sol b/core/test/invariants/LenderInvariants.t.sol index 3d966408..f21d9ea9 100644 --- a/core/test/invariants/LenderInvariants.t.sol +++ b/core/test/invariants/LenderInvariants.t.sol @@ -199,9 +199,9 @@ contract LenderInvariantsTest is Test { address user = lenderHarness.holders(i); // NOTE: As price per share increases (i.e., each share converts to more and more underlying assets), - // this assertion may become flakey due to rounding. Allowing for rounding error of 2 seems sufficient + // this assertion may become flakey due to rounding. Allowing for rounding error of 3 seems sufficient // in our testing. Just make sure the contract itself never assumes principle < underlyingBalance - assertLe(lender.principleOf(user), lender.underlyingBalance(user) + 2); + assertLe(lender.principleOf(user), lender.underlyingBalance(user) + 3); } } @@ -232,4 +232,14 @@ contract LenderInvariantsTest is Test { 1 * count // max error of 1 per user ); } + + function invariant_borrowBalanceIsNonZeroIfUnitsExist() public { + uint256 count = lenderHarness.getBorrowerCount(); + for (uint256 i = 0; i < count; i++) { + address borrower = lenderHarness.borrowers(i); + + if (lender.borrows(borrower) > 1) assertGt(lender.borrowBalanceStored(borrower), 0); + else assertEq(lender.borrowBalanceStored(borrower), 0); + } + } }