From f32bb9c692f4bbee2b9f54eee87329b8a41b514f Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Mon, 6 May 2024 16:35:58 -0400 Subject: [PATCH 01/20] fix: revert to rebasing RewardToken --- src/IonPool.sol | 12 +- src/token/RewardToken.sol | 17 ++- src/vault/Vault.sol | 20 ++- test/fork/fuzz/lrt/EtherFiLibrary.t.sol | 2 +- test/integration/concrete/WeEthIonPool.t.sol | 11 +- test/invariant/IonPool/ActorManager.t.sol | 11 +- test/invariant/RewardToken/ActorManager.t.sol | 8 +- test/invariant/RewardToken/Handlers.t.sol | 8 +- test/unit/concrete/RewardToken.t.sol | 58 ++++---- test/unit/concrete/vault/Vault.t.sol | 140 ++++++++---------- test/unit/fuzz/IonPool.t.sol | 26 ++-- test/unit/fuzz/RewardToken.t.sol | 48 +++--- test/unit/fuzz/vault/Vault.t.sol | 2 +- 13 files changed, 177 insertions(+), 186 deletions(-) diff --git a/src/IonPool.sol b/src/IonPool.sol index 97e53c2a..233c48ca 100644 --- a/src/IonPool.sol +++ b/src/IonPool.sol @@ -364,7 +364,7 @@ contract IonPool is PausableUpgradeable, RewardToken { function _accrueInterest() internal returns (uint256 newTotalDebt) { IonPoolStorage storage $ = _getIonPoolStorage(); - uint256 totalEthSupply = getTotalUnderlyingClaimsUnaccrued(); + uint256 totalEthSupply = totalSupplyUnaccrued(); uint256 totalSupplyFactorIncrease; uint256 totalTreasuryMintAmount; @@ -419,7 +419,7 @@ contract IonPool is PausableUpgradeable, RewardToken { rateIncreases = new uint104[](ilksLength); timestampIncreases = new uint48[](ilksLength); - uint256 totalEthSupply = getTotalUnderlyingClaimsUnaccrued(); + uint256 totalEthSupply = totalSupplyUnaccrued(); for (uint8 i = 0; i < ilksLength;) { ( @@ -457,7 +457,7 @@ contract IonPool is PausableUpgradeable, RewardToken { returns (uint104 newRateIncrease, uint48 timestampIncrease) { (,, newRateIncrease,, timestampIncrease) = - _calculateRewardAndDebtDistributionForIlk(ilkIndex, getTotalUnderlyingClaimsUnaccrued()); + _calculateRewardAndDebtDistributionForIlk(ilkIndex, totalSupplyUnaccrued()); } function _calculateRewardAndDebtDistributionForIlk( @@ -512,7 +512,7 @@ contract IonPool is PausableUpgradeable, RewardToken { newDebtIncrease = _totalNormalizedDebt * newRateIncrease; // [RAD] // Income distribution - uint256 _normalizedTotalSupply = totalSupplyUnaccrued(); // [WAD] + uint256 _normalizedTotalSupply = normalizedTotalSupplyUnaccrued(); // [WAD] // If there is no supply, then nothing is being lent out. supplyFactorIncrease = _normalizedTotalSupply == 0 @@ -570,7 +570,7 @@ contract IonPool is PausableUpgradeable, RewardToken { uint256 _supplyCap = $.supplyCap; - if (getTotalUnderlyingClaims() > _supplyCap) revert DepositSurpassesSupplyCap(amount, _supplyCap); + if (totalSupply() > _supplyCap) revert DepositSurpassesSupplyCap(amount, _supplyCap); emit Supply(user, _msgSender(), amount, _supplyFactor, newTotalDebt); } @@ -954,7 +954,7 @@ contract IonPool is PausableUpgradeable, RewardToken { function getCurrentBorrowRate(uint8 ilkIndex) external view returns (uint256 borrowRate, uint256 reserveFactor) { IonPoolStorage storage $ = _getIonPoolStorage(); - uint256 totalEthSupply = getTotalUnderlyingClaimsUnaccrued(); + uint256 totalEthSupply = totalSupplyUnaccrued(); uint256 _totalNormalizedDebt = $.ilks[ilkIndex].totalNormalizedDebt; uint256 _rate = $.ilks[ilkIndex].rate; diff --git a/src/token/RewardToken.sol b/src/token/RewardToken.sol index 54137b43..d50aae14 100644 --- a/src/token/RewardToken.sol +++ b/src/token/RewardToken.sol @@ -453,7 +453,7 @@ abstract contract RewardToken is * @dev Current token balance * @param user to get balance of */ - function getUnderlyingClaimOf(address user) public view returns (uint256) { + function balanceOf(address user) public view returns (uint256) { RewardTokenStorage storage $ = _getRewardTokenStorage(); (uint256 totalSupplyFactorIncrease,,,,) = calculateRewardAndDebtDistribution(); @@ -465,7 +465,7 @@ abstract contract RewardToken is * @dev Accounting is done in normalized balances * @param user to get normalized balance of */ - function balanceOf(address user) external view returns (uint256) { + function normalizedBalanceOf(address user) external view returns (uint256) { RewardTokenStorage storage $ = _getRewardTokenStorage(); return $._normalizedBalances[user]; } @@ -494,7 +494,10 @@ abstract contract RewardToken is return $.treasury; } - function getTotalUnderlyingClaimsUnaccrued() public view returns (uint256) { + /** + * @dev Total claim of the underlying asset belonging to lenders not inclusive of the new interest to be accrued. + */ + function totalSupplyUnaccrued() public view returns (uint256) { RewardTokenStorage storage $ = _getRewardTokenStorage(); uint256 _normalizedTotalSupply = $.normalizedTotalSupply; @@ -507,9 +510,9 @@ abstract contract RewardToken is } /** - * @dev Total claim of the underlying asset belonging to lenders. + * @dev Total claim of the underlying asset belonging to lender inclusive of the new interest to be accrued. */ - function getTotalUnderlyingClaims() public view returns (uint256) { + function totalSupply() public view returns (uint256) { RewardTokenStorage storage $ = _getRewardTokenStorage(); uint256 _normalizedTotalSupply = $.normalizedTotalSupply; @@ -523,7 +526,7 @@ abstract contract RewardToken is return _normalizedTotalSupply.rayMulDown($.supplyFactor + totalSupplyFactorIncrease); } - function totalSupplyUnaccrued() public view returns (uint256) { + function normalizedTotalSupplyUnaccrued() public view returns (uint256) { RewardTokenStorage storage $ = _getRewardTokenStorage(); return $.normalizedTotalSupply; } @@ -533,7 +536,7 @@ abstract contract RewardToken is * * Normalized total supply and total supply are same in non-rebasing token. */ - function totalSupply() public view returns (uint256) { + function normalizedTotalSupply() public view returns (uint256) { RewardTokenStorage storage $ = _getRewardTokenStorage(); (uint256 totalSupplyFactorIncrease, uint256 totalTreasuryMintAmount,,,) = calculateRewardAndDebtDistribution(); diff --git a/src/vault/Vault.sol b/src/vault/Vault.sol index fc3231db..33d19436 100644 --- a/src/vault/Vault.sol +++ b/src/vault/Vault.sol @@ -197,10 +197,10 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy if (pool == IDLE) { if (BASE_ASSET.balanceOf(address(this)) != 0) revert InvalidIdleMarketRemovalNonZeroBalance(); } else { - // Checks `balanceOf` as it may be possible that - // `getUnderlyingClaimOf` returns zero even though the + // Checks `normalizedBalanceOf` as it may be possible that + // `balanceOf` returns zero even though the // `normalizedBalance` is zero. - if (pool.balanceOf(address(this)) != 0) revert InvalidMarketRemovalNonZeroSupply(pool); + if (pool.normalizedBalanceOf(address(this)) != 0) revert InvalidMarketRemovalNonZeroSupply(pool); BASE_ASSET.approve(address(pool), 0); } @@ -323,7 +323,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy MarketAllocation calldata allocation = allocations[i]; IIonPool pool = allocation.pool; - uint256 currentSupplied = pool == IDLE ? currentIdleDeposits : pool.getUnderlyingClaimOf(address(this)); + uint256 currentSupplied = pool == IDLE ? currentIdleDeposits : pool.balanceOf(address(this)); int256 assets = allocation.assets; // to deposit or withdraw // if `assets` is `type(int256).min`, this means fully withdraw from the market. @@ -645,7 +645,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy /** * @notice Returns the total claim that the vault has across all supported IonPools. - * @dev `IonPool.getUnderlyingClaimOf` returns the rebasing balance of the + * @dev `IonPool.balanceOf` returns the rebasing balance of the * lender receipt token that is pegged 1:1 to the underlying supplied asset. * @return assets The total assets held on the contract and inside the underlying * pools by this vault. @@ -655,8 +655,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy for (uint256 i; i != _supportedMarketsLength;) { IIonPool pool = IIonPool(supportedMarkets.at(i)); - uint256 assetsInPool = - pool == IDLE ? BASE_ASSET.balanceOf(address(this)) : pool.getUnderlyingClaimOf(address(this)); + uint256 assetsInPool = pool == IDLE ? BASE_ASSET.balanceOf(address(this)) : pool.balanceOf(address(this)); assets += assetsInPool; @@ -897,7 +896,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy * @return The max amount of assets withdrawable from this IonPool. */ function _withdrawable(IIonPool pool) internal view returns (uint256) { - uint256 currentSupplied = pool.getUnderlyingClaimOf(address(this)); + uint256 currentSupplied = pool.balanceOf(address(this)); // TODO should be balanceOf uint256 availableLiquidity = uint256(pool.extsload(ION_POOL_LIQUIDITY_SLOT)); return Math.min(currentSupplied, availableLiquidity); @@ -910,9 +909,8 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy * @return The max amount of assets depositable to this IonPool. */ function _depositable(IIonPool pool) internal view returns (uint256) { - uint256 allocationCapDiff = _zeroFloorSub(caps[pool], pool.getUnderlyingClaimOf(address(this))); - uint256 supplyCapDiff = - _zeroFloorSub(uint256(pool.extsload(ION_POOL_SUPPLY_CAP_SLOT)), pool.getTotalUnderlyingClaims()); + uint256 allocationCapDiff = _zeroFloorSub(caps[pool], pool.balanceOf(address(this))); + uint256 supplyCapDiff = _zeroFloorSub(uint256(pool.extsload(ION_POOL_SUPPLY_CAP_SLOT)), pool.totalSupply()); return Math.min(allocationCapDiff, supplyCapDiff); } diff --git a/test/fork/fuzz/lrt/EtherFiLibrary.t.sol b/test/fork/fuzz/lrt/EtherFiLibrary.t.sol index ce74d121..1152a078 100644 --- a/test/fork/fuzz/lrt/EtherFiLibrary.t.sol +++ b/test/fork/fuzz/lrt/EtherFiLibrary.t.sol @@ -31,7 +31,7 @@ contract EtherFiLibrary_FuzzTest is Test { function testForkFuzz_GetLstAmountOutForEthAmountIn(uint256 ethAmount) external { vm.assume(ethAmount != 0); - vm.assume(ethAmount < type(uint128).max); + vm.assume(ethAmount < type(uint96).max); uint256 lrtAmountOut = EtherFiLibrary.getLstAmountOutForEthAmountIn(WEETH_ADDRESS, ethAmount); diff --git a/test/integration/concrete/WeEthIonPool.t.sol b/test/integration/concrete/WeEthIonPool.t.sol index 6b99f878..ab566eb9 100644 --- a/test/integration/concrete/WeEthIonPool.t.sol +++ b/test/integration/concrete/WeEthIonPool.t.sol @@ -125,7 +125,7 @@ contract WeEthIonPool_IntegrationTest is WeEthIonPoolSharedSetup { ionPool.supply(lenderA, lenderAFirstSupplyAmount, lenderProofs[0]); vm.stopPrank(); - assertEq(ionPool.getUnderlyingClaimOf(lenderA), lenderAFirstSupplyAmount, "lender balance after 1st supply"); + assertEq(ionPool.balanceOf(lenderA), lenderAFirstSupplyAmount, "lender balance after 1st supply"); assertEq(lens.liquidity(iIonPool), lenderAFirstSupplyAmount, "liquidity after 1st supply"); /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ @@ -172,10 +172,7 @@ contract WeEthIonPool_IntegrationTest is WeEthIonPoolSharedSetup { uint256 roundingError = ionPool.supplyFactor() / 1e27; assertApproxEqAbs( - ionPool.getUnderlyingClaimOf(lenderB), - lenderBFirstSupplyAmount, - roundingError, - "lenderB balance after 1st supply" + ionPool.balanceOf(lenderB), lenderBFirstSupplyAmount, roundingError, "lenderB balance after 1st supply" ); /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ @@ -219,14 +216,14 @@ contract WeEthIonPool_IntegrationTest is WeEthIonPoolSharedSetup { vm.startPrank(lenderA); ionPool.withdraw(lenderA, lender1WithdrawAmountFail); - uint256 lenderABalanceBefore = ionPool.getUnderlyingClaimOf(lenderA); + uint256 lenderABalanceBefore = ionPool.balanceOf(lenderA); uint256 lender1WithdrawAmount = 10e18; ionPool.withdraw(lenderA, lender1WithdrawAmount); vm.stopPrank(); assertEq( - ionPool.getUnderlyingClaimOf(lenderA), + ionPool.balanceOf(lenderA), lenderABalanceBefore - lender1WithdrawAmount, "lenderA balance after 1st withdrawal" ); diff --git a/test/invariant/IonPool/ActorManager.t.sol b/test/invariant/IonPool/ActorManager.t.sol index ab85cda2..ea0f94fd 100644 --- a/test/invariant/IonPool/ActorManager.t.sol +++ b/test/invariant/IonPool/ActorManager.t.sol @@ -227,7 +227,7 @@ contract IonPool_InvariantTest is IonPoolSharedSetup { function invariant_LenderDepositsAddToBalance() external returns (bool) { for (uint256 i = 0; i < lenders.length; i++) { - assertEq(lenders[i].totalHoldingsNormalized(), ionPool.balanceOf(address(lenders[i]))); + assertEq(lenders[i].totalHoldingsNormalized(), ionPool.normalizedBalanceOf(address(lenders[i]))); } return !failed(); @@ -236,9 +236,12 @@ contract IonPool_InvariantTest is IonPoolSharedSetup { function invariant_LenderBalancesPlusTreasuryAddToTotalSupply() external returns (bool) { uint256 totalLenderNormalizedBalances; for (uint256 i = 0; i < lenders.length; i++) { - totalLenderNormalizedBalances += ionPool.balanceOf(address(lenders[i])); + totalLenderNormalizedBalances += ionPool.normalizedBalanceOf(address(lenders[i])); } - assertEq(totalLenderNormalizedBalances + ionPool.balanceOf(TREASURY), ionPool.totalSupplyUnaccrued()); + assertEq( + totalLenderNormalizedBalances + ionPool.normalizedBalanceOf(TREASURY), + ionPool.normalizedTotalSupplyUnaccrued() + ); return !failed(); } @@ -256,7 +259,7 @@ contract IonPool_InvariantTest is IonPoolSharedSetup { assertGe(lens.liquidity(iIonPool) + totalDebt, ionPool.totalSupplyUnaccrued()); assertGe( lens.liquidity(iIonPool).scaleUpToRad(18) + lens.debtUnaccrued(iIonPool), - ionPool.totalSupplyUnaccrued() * ionPool.supplyFactorUnaccrued() + ionPool.normalizedTotalSupplyUnaccrued() * ionPool.supplyFactorUnaccrued() ); return !failed(); diff --git a/test/invariant/RewardToken/ActorManager.t.sol b/test/invariant/RewardToken/ActorManager.t.sol index d759dab6..f15bfb94 100644 --- a/test/invariant/RewardToken/ActorManager.t.sol +++ b/test/invariant/RewardToken/ActorManager.t.sol @@ -105,20 +105,20 @@ contract RewardToken_InvariantTest is RewardTokenSharedSetup { uint256 totalSupplyByBalances; for (uint256 i = 0; i < userHandlers.length; i++) { UserHandler user = userHandlers[i]; - totalSupplyByBalances += rewardModule.balanceOf(address(user)); + totalSupplyByBalances += rewardModule.normalizedBalanceOf(address(user)); } underlying.balanceOf(address(rewardModule)); // update underlying balance rewardModule.totalSupply(); - assertEq(rewardModule.totalSupply(), totalSupplyByBalances); + assertEq(rewardModule.normalizedTotalSupply(), totalSupplyByBalances); } function invariant_lenderClaimAlwaysBacked() external { - uint256 lenderClaim = rewardModule.getTotalUnderlyingClaims(); + uint256 totalSupply = rewardModule.totalSupply(); uint256 underlyingBalance = underlying.balanceOf(address(rewardModule)); - assertGe(underlyingBalance, lenderClaim); + assertGe(underlyingBalance, totalSupply); } } diff --git a/test/invariant/RewardToken/Handlers.t.sol b/test/invariant/RewardToken/Handlers.t.sol index 57064718..eb89fd80 100644 --- a/test/invariant/RewardToken/Handlers.t.sol +++ b/test/invariant/RewardToken/Handlers.t.sol @@ -39,11 +39,11 @@ contract UserHandler is Handler { } function burn(address account, uint256 amount) external { - amount = bound(amount, 0, REWARD_MODULE.getUnderlyingClaimOf(account)); + amount = bound(amount, 0, REWARD_MODULE.balanceOf(account)); uint256 currentSupplyFactor = REWARD_MODULE.supplyFactor(); uint256 amountNormalized = amount.rayDivUp(currentSupplyFactor); - if (amountNormalized == 0 || amountNormalized > REWARD_MODULE.balanceOf(account)) return; + if (amountNormalized == 0 || amountNormalized > REWARD_MODULE.normalizedBalanceOf(account)) return; REWARD_MODULE.burn(account, account, amount); } @@ -66,11 +66,11 @@ contract SupplyFactorIncreaseHandler is Handler { uint256 oldSupplyFactor = REWARD_MODULE.supplyFactor(); amount = bound(amount, 1.1e27, 1.25e27); // between 1E-16 and 15% - uint256 oldTotalSupply = REWARD_MODULE.getTotalUnderlyingClaims(); + uint256 oldTotalSupply = REWARD_MODULE.totalSupply(); uint256 newSupplyFactor = oldSupplyFactor.rayMulDown(amount); REWARD_MODULE.setSupplyFactor(newSupplyFactor); - uint256 interestCreated = REWARD_MODULE.getTotalUnderlyingClaims() - oldTotalSupply; + uint256 interestCreated = REWARD_MODULE.totalSupply() - oldTotalSupply; UNDERLYING.mint(address(REWARD_MODULE), interestCreated + 1); } } diff --git a/test/unit/concrete/RewardToken.t.sol b/test/unit/concrete/RewardToken.t.sol index 32f79461..134448a4 100644 --- a/test/unit/concrete/RewardToken.t.sol +++ b/test/unit/concrete/RewardToken.t.sol @@ -42,7 +42,7 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { rewardModule.mint(address(0), amountOfRewards); rewardModule.mint(address(this), amountOfRewards); - assertEq(rewardModule.balanceOf(address(this)), amountOfRewards); + assertEq(rewardModule.normalizedBalanceOf(address(this)), amountOfRewards); assertEq(underlying.balanceOf(address(this)), INITIAL_UNDERYLING - amountOfRewards); assertEq(underlying.balanceOf(address(rewardModule)), amountOfRewards); } @@ -66,14 +66,14 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { underlying.approve(address(rewardModule), INITIAL_UNDERYLING); rewardModule.mint(address(this), amountOfRewards); - assertEq(rewardModule.balanceOf(address(this)), amountOfRewards); + assertEq(rewardModule.normalizedBalanceOf(address(this)), amountOfRewards); assertEq(underlying.balanceOf(address(this)), INITIAL_UNDERYLING - amountOfRewards); vm.expectRevert(abi.encodeWithSelector(RewardToken.InvalidSender.selector, address(0))); rewardModule.burn(address(0), address(this), amountOfRewards); rewardModule.burn(address(this), address(this), amountOfRewards); - assertEq(rewardModule.balanceOf(address(this)), 0); + assertEq(rewardModule.normalizedBalanceOf(address(this)), 0); } function test_MintRewardWithSupplyFactorChange() external { @@ -85,8 +85,8 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { uint256 expectedNormalizedMint1 = amountOfRewards.rayDivDown(supplyFactorOld); - assertEq(rewardModule.balanceOf(address(this)), expectedNormalizedMint1); - assertEq(rewardModule.getUnderlyingClaimOf(address(this)), amountOfRewards); + assertEq(rewardModule.normalizedBalanceOf(address(this)), expectedNormalizedMint1); + assertEq(rewardModule.balanceOf(address(this)), amountOfRewards); assertEq(underlying.balanceOf(address(this)), INITIAL_UNDERYLING - amountOfRewards); assertEq(underlying.balanceOf(address(rewardModule)), amountOfRewards); @@ -104,8 +104,8 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { uint256 totalDepositsNormalized = expectedNormalizedMint1 + expectedNormalizedMint2; uint256 totalValue = totalDepositsNormalized.rayMulDown(supplyFactorNew); - assertEq(rewardModule.balanceOf(address(this)), totalDepositsNormalized); - assertEq(rewardModule.getUnderlyingClaimOf(address(this)), totalValue); + assertEq(rewardModule.normalizedBalanceOf(address(this)), totalDepositsNormalized); + assertEq(rewardModule.balanceOf(address(this)), totalValue); assertEq(underlying.balanceOf(address(this)), INITIAL_UNDERYLING - totalDeposited); assertEq(underlying.balanceOf(address(rewardModule)), totalDeposited + interestCreated); @@ -129,8 +129,8 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { uint256 expectedNormalizedMint1 = amountOfRewards.rayDivDown(supplyFactorOld); - assertEq(rewardModule.balanceOf(address(this)), expectedNormalizedMint1); - assertEq(rewardModule.getUnderlyingClaimOf(address(this)), amountOfRewards); + assertEq(rewardModule.normalizedBalanceOf(address(this)), expectedNormalizedMint1); + assertEq(rewardModule.balanceOf(address(this)), amountOfRewards); assertEq(underlying.balanceOf(address(this)), INITIAL_UNDERYLING - amountOfRewards); assertEq(underlying.balanceOf(address(rewardModule)), amountOfRewards); @@ -148,8 +148,8 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { uint256 totalDepositsNormalized = expectedNormalizedMint1 + expectedNormalizedMint2; uint256 totalValue = totalDepositsNormalized.rayMulDown(supplyFactorNew); - assertEq(rewardModule.balanceOf(address(this)), totalDepositsNormalized); - assertEq(rewardModule.getUnderlyingClaimOf(address(this)), totalValue); + assertEq(rewardModule.normalizedBalanceOf(address(this)), totalDepositsNormalized); + assertEq(rewardModule.balanceOf(address(this)), totalValue); assertEq(underlying.balanceOf(address(this)), INITIAL_UNDERYLING - totalDeposited); assertEq(underlying.balanceOf(address(rewardModule)), totalDeposited + interestCreated); @@ -174,9 +174,13 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { rewardModule.burn(address(this), address(this), burnAmount); - assertEq(rewardModule.getUnderlyingClaimOf(address(this)), totalValue - burnAmount); - assertEq(rewardModule.totalSupply(), totalDepositsNormalized - burnAmountNormalized, "total supply after burn"); - assertEq(rewardModule.balanceOf(address(this)), totalDepositsNormalized - burnAmountNormalized); + assertEq(rewardModule.balanceOf(address(this)), totalValue - burnAmount); + assertEq( + rewardModule.normalizedTotalSupply(), + totalDepositsNormalized - burnAmountNormalized, + "total supply after burn" + ); + assertEq(rewardModule.normalizedBalanceOf(address(this)), totalDepositsNormalized - burnAmountNormalized); assertEq(underlying.balanceOf(address(this)), INITIAL_UNDERYLING - totalDeposited + burnAmount); assertEq(underlying.balanceOf(address(rewardModule)), totalDeposited + interestCreated - burnAmount); } @@ -187,7 +191,7 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { underlying.approve(address(rewardModule), INITIAL_UNDERYLING); rewardModule.mint(address(this), amountOfRewardTokens); - assertEq(rewardModule.balanceOf(address(this)), amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(address(this)), amountOfRewardTokens); assertEq(underlying.balanceOf(address(this)), INITIAL_UNDERYLING - amountOfRewardTokens); assertEq(underlying.balanceOf(address(rewardModule)), amountOfRewardTokens); @@ -206,8 +210,8 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { rewardModule.transfer(address(this), amountOfRewardTokens); rewardModule.transfer(receivingUser, amountOfRewardTokens); - assertEq(rewardModule.balanceOf(address(this)), 0); - assertEq(rewardModule.balanceOf(receivingUser), amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(address(this)), 0); + assertEq(rewardModule.normalizedBalanceOf(receivingUser), amountOfRewardTokens); assertEq(underlying.balanceOf(address(this)), INITIAL_UNDERYLING - amountOfRewardTokens); } @@ -217,8 +221,8 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { underlying.approve(address(rewardModule), INITIAL_UNDERYLING); rewardModule.mint(sendingUser, amountOfRewardTokens); - assertEq(rewardModule.balanceOf(sendingUser), amountOfRewardTokens); - assertEq(rewardModule.balanceOf(receivingUser), 0); + assertEq(rewardModule.normalizedBalanceOf(sendingUser), amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(receivingUser), 0); assertEq(rewardModule.allowance(sendingUser, spender), 0); assertEq(underlying.balanceOf(address(this)), INITIAL_UNDERYLING - amountOfRewardTokens); assertEq(underlying.balanceOf(address(rewardModule)), amountOfRewardTokens); @@ -246,8 +250,8 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { rewardModule.transferFrom(sendingUser, receivingUser, amountOfRewardTokens + 1); rewardModule.transferFrom(sendingUser, receivingUser, amountOfRewardTokens); - assertEq(rewardModule.balanceOf(sendingUser), 0); - assertEq(rewardModule.balanceOf(receivingUser), amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(sendingUser), 0); + assertEq(rewardModule.normalizedBalanceOf(receivingUser), amountOfRewardTokens); assertEq(rewardModule.allowance(sendingUser, spender), 0); } @@ -257,8 +261,8 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { underlying.approve(address(rewardModule), INITIAL_UNDERYLING); rewardModule.mint(sendingUser, amountOfRewardTokens); - assertEq(rewardModule.balanceOf(sendingUser), amountOfRewardTokens); - assertEq(rewardModule.balanceOf(receivingUser), 0); + assertEq(rewardModule.normalizedBalanceOf(sendingUser), amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(receivingUser), 0); assertEq(rewardModule.allowance(sendingUser, spender), 0); assertEq(underlying.balanceOf(address(this)), INITIAL_UNDERYLING - amountOfRewardTokens); assertEq(underlying.balanceOf(address(rewardModule)), amountOfRewardTokens); @@ -307,8 +311,8 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { rewardModule.transferFrom(sendingUser, receivingUser, amountOfRewardTokens + 1); rewardModule.transferFrom(sendingUser, receivingUser, amountOfRewardTokens); - assertEq(rewardModule.balanceOf(sendingUser), 0); - assertEq(rewardModule.balanceOf(receivingUser), amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(sendingUser), 0); + assertEq(rewardModule.normalizedBalanceOf(receivingUser), amountOfRewardTokens); assertEq(rewardModule.allowance(sendingUser, spender), 0); } @@ -318,8 +322,8 @@ contract RewardToken_UnitTest is RewardTokenSharedSetup { underlying.approve(address(rewardModule), INITIAL_UNDERYLING); rewardModule.mint(sendingUser, amountOfRewardTokens); - assertEq(rewardModule.balanceOf(sendingUser), amountOfRewardTokens); - assertEq(rewardModule.balanceOf(receivingUser), 0); + assertEq(rewardModule.normalizedBalanceOf(sendingUser), amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(receivingUser), 0); assertEq(rewardModule.allowance(sendingUser, spender), 0); assertEq(underlying.balanceOf(address(this)), INITIAL_UNDERYLING - amountOfRewardTokens); assertEq(underlying.balanceOf(address(rewardModule)), amountOfRewardTokens); diff --git a/test/unit/concrete/vault/Vault.t.sol b/test/unit/concrete/vault/Vault.t.sol index cafcdb22..bb73be56 100644 --- a/test/unit/concrete/vault/Vault.t.sol +++ b/test/unit/concrete/vault/Vault.t.sol @@ -298,7 +298,7 @@ contract VaultSetUpTest is VaultSharedSetup { vault.deposit(depositAmount, address(this)); - assertGt(weEthIonPool.balanceOf(address(vault)), 0, "deposited to weEthIonPool"); + assertGt(weEthIonPool.normalizedBalanceOf(address(vault)), 0, "deposited to weEthIonPool"); vm.prank(OWNER); vm.expectRevert(abi.encodeWithSelector(Vault.InvalidMarketRemovalNonZeroSupply.selector, weEthIonPool)); @@ -363,7 +363,7 @@ contract VaultSetUpTest is VaultSharedSetup { setERC20Balance(address(BASE_ASSET), address(this), depositAmount); vault.deposit(depositAmount, address(this)); - assertGt(weEthIonPool.balanceOf(address(vault)), 0, "deposited to weEthIonPool"); + assertGt(weEthIonPool.normalizedBalanceOf(address(vault)), 0, "deposited to weEthIonPool"); bytes memory reallocateCalldata = abi.encodeWithSelector(Vault.reallocate.selector, allocs); @@ -616,7 +616,7 @@ abstract contract VaultDeposit is VaultSharedSetup { updateSupplyCaps(vault, type(uint256).max, type(uint256).max, type(uint256).max); updateAllocationCaps(vault, type(uint256).max, type(uint256).max, type(uint256).max); - uint256 prevWeEthShares = weEthIonPool.balanceOf(address(vault)); + uint256 prevWeEthShares = weEthIonPool.normalizedBalanceOf(address(vault)); vault.deposit(depositAmount, address(this)); @@ -624,7 +624,7 @@ abstract contract VaultDeposit is VaultSharedSetup { assertEq(vault.balanceOf(address(this)), depositAmount, "user vault shares balance"); assertEq(BASE_ASSET.balanceOf(address(vault)), 0, "base asset balance should be zero"); assertEq( - weEthIonPool.getUnderlyingClaimOf(address(vault)), + weEthIonPool.balanceOf(address(vault)), claimAfterDeposit(prevWeEthShares, depositAmount, weEthIonPool.supplyFactor()), "vault iToken claim" ); @@ -638,9 +638,9 @@ abstract contract VaultDeposit is VaultSharedSetup { updateSupplyCaps(vault, type(uint256).max, type(uint256).max, type(uint256).max); updateAllocationCaps(vault, 1e18, 1e18, 1e18); - uint256 prevWeEthShares = weEthIonPool.balanceOf(address(vault)); - uint256 prevRsEthShares = rsEthIonPool.balanceOf(address(vault)); - uint256 prevRswEthShares = rswEthIonPool.balanceOf(address(vault)); + uint256 prevWeEthShares = weEthIonPool.normalizedBalanceOf(address(vault)); + uint256 prevRsEthShares = rsEthIonPool.normalizedBalanceOf(address(vault)); + uint256 prevRswEthShares = rswEthIonPool.normalizedBalanceOf(address(vault)); // 3e18 gets spread out equally amongst the three pools vault.deposit(depositAmount, address(this)); @@ -650,17 +650,17 @@ abstract contract VaultDeposit is VaultSharedSetup { assertEq(BASE_ASSET.balanceOf(address(vault)), 0, "base asset balance should be zero"); assertEq( - weEthIonPool.getUnderlyingClaimOf(address(vault)), + weEthIonPool.balanceOf(address(vault)), claimAfterDeposit(prevWeEthShares, 1e18, weEthIonPool.supplyFactor()), "weEth vault iToken claim" ); assertEq( - rsEthIonPool.getUnderlyingClaimOf(address(vault)), + rsEthIonPool.balanceOf(address(vault)), claimAfterDeposit(prevRsEthShares, 1e18, rsEthIonPool.supplyFactor()), "rsEth vault iToken claim" ); assertEq( - rswEthIonPool.getUnderlyingClaimOf(address(vault)), + rswEthIonPool.balanceOf(address(vault)), claimAfterDeposit(prevRswEthShares, 1e18, rswEthIonPool.supplyFactor()), "rswEth vault iToken claim" ); @@ -674,9 +674,9 @@ abstract contract VaultDeposit is VaultSharedSetup { updateSupplyCaps(vault, type(uint256).max, type(uint256).max, type(uint256).max); updateAllocationCaps(vault, 3e18, 5e18, 7e18); - uint256 prevWeEthShares = weEthIonPool.balanceOf(address(vault)); - uint256 prevRsEthShares = rsEthIonPool.balanceOf(address(vault)); - uint256 prevRswEthShares = rswEthIonPool.balanceOf(address(vault)); + uint256 prevWeEthShares = weEthIonPool.normalizedBalanceOf(address(vault)); + uint256 prevRsEthShares = rsEthIonPool.normalizedBalanceOf(address(vault)); + uint256 prevRswEthShares = rswEthIonPool.normalizedBalanceOf(address(vault)); // 3e18 gets spread out equally amongst the three pools vault.deposit(depositAmount, address(this)); @@ -686,17 +686,17 @@ abstract contract VaultDeposit is VaultSharedSetup { assertEq(BASE_ASSET.balanceOf(address(vault)), 0, "base asset balance should be zero"); assertEq( - weEthIonPool.getUnderlyingClaimOf(address(vault)), + weEthIonPool.balanceOf(address(vault)), claimAfterDeposit(prevWeEthShares, 2e18, weEthIonPool.supplyFactor()), "weEth vault iToken claim" ); assertEq( - rsEthIonPool.getUnderlyingClaimOf(address(vault)), + rsEthIonPool.balanceOf(address(vault)), claimAfterDeposit(prevRsEthShares, 3e18, rsEthIonPool.supplyFactor()), "rsEth vault iToken claim" ); assertEq( - rswEthIonPool.getUnderlyingClaimOf(address(vault)), + rswEthIonPool.balanceOf(address(vault)), claimAfterDeposit(prevRswEthShares, 5e18, rswEthIonPool.supplyFactor()), "rswEth vault iToken claim" ); @@ -712,9 +712,9 @@ abstract contract VaultDeposit is VaultSharedSetup { updateSupplyCaps(vault, 3e18, 10e18, 5e18); updateAllocationCaps(vault, 5e18, 7e18, 20e18); - uint256 prevWeEthShares = weEthIonPool.balanceOf(address(vault)); - uint256 prevRsEthShares = rsEthIonPool.balanceOf(address(vault)); - uint256 prevRswEthShares = rswEthIonPool.balanceOf(address(vault)); + uint256 prevWeEthShares = weEthIonPool.normalizedBalanceOf(address(vault)); + uint256 prevRsEthShares = rsEthIonPool.normalizedBalanceOf(address(vault)); + uint256 prevRswEthShares = rswEthIonPool.normalizedBalanceOf(address(vault)); vault.deposit(depositAmount, address(this)); @@ -723,17 +723,17 @@ abstract contract VaultDeposit is VaultSharedSetup { assertEq(BASE_ASSET.balanceOf(address(vault)), 0, "base asset balance should be zero"); assertEq( - weEthIonPool.getUnderlyingClaimOf(address(vault)), + weEthIonPool.balanceOf(address(vault)), claimAfterDeposit(prevWeEthShares, 2e18, weEthIonPool.supplyFactor()), "weEth vault iToken claim" ); assertEq( - rsEthIonPool.getUnderlyingClaimOf(address(vault)), + rsEthIonPool.balanceOf(address(vault)), claimAfterDeposit(prevRsEthShares, 3e18, rsEthIonPool.supplyFactor()), "rsEth vault iToken claim" ); assertEq( - rswEthIonPool.getUnderlyingClaimOf(address(vault)), + rswEthIonPool.balanceOf(address(vault)), claimAfterDeposit(prevRswEthShares, 7e18, rswEthIonPool.supplyFactor()), "rswEth vault iToken claim" ); @@ -839,9 +839,7 @@ abstract contract VaultWithdraw is VaultSharedSetup { ); assertEq(vault.totalSupply(), expectedNewTotalSupply, "vault shares total supply"); - assertEq( - vault.totalAssets(), rsEthIonPool.getUnderlyingClaimOf(address(vault)), "single market for total assets" - ); + assertEq(vault.totalAssets(), rsEthIonPool.balanceOf(address(vault)), "single market for total assets"); assertEq(BASE_ASSET.balanceOf(address(vault)), 0, "valt's base asset balance should be zero"); // user @@ -900,10 +898,10 @@ abstract contract VaultWithdraw is VaultSharedSetup { ); assertEq(vault.totalSupply(), expectedNewTotalSupply, "vault shares total supply"); - assertEq(rsEthIonPool.getUnderlyingClaimOf(address(vault)), 0, "vault pool1 balance"); - assertEq(rswEthIonPool.getUnderlyingClaimOf(address(vault)), 0, "vault pool2 balance"); + assertEq(rsEthIonPool.balanceOf(address(vault)), 0, "vault pool1 balance"); + assertEq(rswEthIonPool.balanceOf(address(vault)), 0, "vault pool2 balance"); assertLe( - expectedNewTotalAssets - weEthIonPool.getUnderlyingClaimOf(address(vault)), + expectedNewTotalAssets - weEthIonPool.balanceOf(address(vault)), weEthIonPool.supplyFactor() / RAY, "vault pool3 balance" ); @@ -962,9 +960,9 @@ abstract contract VaultReallocate is VaultSharedSetup { uint256 prevTotalAssets = vault.totalAssets(); - uint256 prevRsEthClaim = rsEthIonPool.getUnderlyingClaimOf(address(vault)); - uint256 prevRswEthClaim = rswEthIonPool.getUnderlyingClaimOf(address(vault)); - uint256 prevWeEthClaim = weEthIonPool.getUnderlyingClaimOf(address(vault)); + uint256 prevRsEthClaim = rsEthIonPool.balanceOf(address(vault)); + uint256 prevRswEthClaim = rswEthIonPool.balanceOf(address(vault)); + uint256 prevWeEthClaim = weEthIonPool.balanceOf(address(vault)); int256 rswEthDiff = -1e18; int256 weEthDiff = -2e18; @@ -988,19 +986,19 @@ abstract contract VaultReallocate is VaultSharedSetup { uint256 newTotalAssets = vault.totalAssets(); assertApproxEqAbs( - rsEthIonPool.getUnderlyingClaimOf(address(vault)), + rsEthIonPool.balanceOf(address(vault)), expNewRsEthClaim, rsEthIonPool.supplyFactor() / RAY, "rsEth vault iToken claim" ); assertApproxEqAbs( - rswEthIonPool.getUnderlyingClaimOf(address(vault)), + rswEthIonPool.balanceOf(address(vault)), expNewRswEthClaim, rswEthIonPool.supplyFactor() / RAY, "rswEth vault iToken claim" ); assertApproxEqAbs( - weEthIonPool.getUnderlyingClaimOf(address(vault)), + weEthIonPool.balanceOf(address(vault)), expNewWeEthClaim, weEthIonPool.supplyFactor() / RAY, "weEth vault iToken claim" @@ -1028,9 +1026,9 @@ abstract contract VaultReallocate is VaultSharedSetup { updateAllocationCaps(vault, type(uint256).max, type(uint256).max, type(uint256).max); - uint256 prevWeEthClaim = weEthIonPool.getUnderlyingClaimOf(address(vault)); - uint256 prevRswEthClaim = rswEthIonPool.getUnderlyingClaimOf(address(vault)); - uint256 prevRsEthClaim = rsEthIonPool.getUnderlyingClaimOf(address(vault)); + uint256 prevWeEthClaim = weEthIonPool.balanceOf(address(vault)); + uint256 prevRswEthClaim = rswEthIonPool.balanceOf(address(vault)); + uint256 prevRsEthClaim = rsEthIonPool.balanceOf(address(vault)); uint256 expRswEthClaim = prevRswEthClaim + prevWeEthClaim + prevRsEthClaim; uint256 prevTotalAssets = vault.totalAssets(); @@ -1052,10 +1050,10 @@ abstract contract VaultReallocate is VaultSharedSetup { uint256 newTotalAssets = vault.totalAssets(); - assertEq(rsEthIonPool.getUnderlyingClaimOf(address(vault)), 0, "rsEth vault iToken claim"); - assertEq(weEthIonPool.getUnderlyingClaimOf(address(vault)), 0, "weEth vault iToken claim"); + assertEq(rsEthIonPool.balanceOf(address(vault)), 0, "rsEth vault iToken claim"); + assertEq(weEthIonPool.balanceOf(address(vault)), 0, "weEth vault iToken claim"); assertApproxEqAbs( - rswEthIonPool.getUnderlyingClaimOf(address(vault)), + rswEthIonPool.balanceOf(address(vault)), expRswEthClaim, rswEthIonPool.supplyFactor() / RAY, "rswEth vault iToken claim" @@ -1081,7 +1079,7 @@ abstract contract VaultReallocate is VaultSharedSetup { uint256 prevTotalAssets = vault.totalAssets(); - uint256 weEthCurrentSupplied = weEthIonPool.getUnderlyingClaimOf(address(vault)); + uint256 weEthCurrentSupplied = weEthIonPool.balanceOf(address(vault)); // tries to deposit 2e18 + 2e18 to 3e18 allocation cap Vault.MarketAllocation[] memory allocs = new Vault.MarketAllocation[](3); @@ -1173,20 +1171,14 @@ abstract contract VaultWithIdlePool is VaultSharedSetup { // rsEthIonPool should be full at 30e18 // rswEthIonPool should be at 10e18 assertLe( - 10e18 - weEthIonPool.getUnderlyingClaimOf(address(vault)), - postDepositClaimRE(10e18, weEthIonPoolSF), - "weEthIonPool" + 10e18 - weEthIonPool.balanceOf(address(vault)), postDepositClaimRE(10e18, weEthIonPoolSF), "weEthIonPool" ); assertEq(BASE_ASSET.balanceOf(address(vault)), 20e18, "IDLE"); assertLe( - 30e18 - rsEthIonPool.getUnderlyingClaimOf(address(vault)), - postDepositClaimRE(30e18, rsEthIonPoolSF), - "rsEthIonPool" + 30e18 - rsEthIonPool.balanceOf(address(vault)), postDepositClaimRE(30e18, rsEthIonPoolSF), "rsEthIonPool" ); assertLe( - 10e18 - rswEthIonPool.getUnderlyingClaimOf(address(vault)), - postDepositClaimRE(10e18, rswEthIonPoolSF), - "rswEthIonPool" + 10e18 - rswEthIonPool.balanceOf(address(vault)), postDepositClaimRE(10e18, rswEthIonPoolSF), "rswEthIonPool" ); assertEq(BASE_ASSET.balanceOf(address(this)), 0, "user balance"); } @@ -1221,18 +1213,18 @@ abstract contract VaultWithIdlePool is VaultSharedSetup { assertEq(remainingAssets, 0, "test variables"); assertLe( - expectedWeEthIonPoolClaim - weEthIonPool.getUnderlyingClaimOf(address(vault)), + expectedWeEthIonPoolClaim - weEthIonPool.balanceOf(address(vault)), postDepositClaimRE(expectedWeEthIonPoolClaim, weEthIonPoolSF), "weEthIonPool" ); assertEq(BASE_ASSET.balanceOf(address(vault)), expectedIdleClaim, "IDLE"); assertLe( - expectedRsEthIonPoolClaim - rsEthIonPool.getUnderlyingClaimOf(address(vault)), + expectedRsEthIonPoolClaim - rsEthIonPool.balanceOf(address(vault)), postDepositClaimRE(expectedWeEthIonPoolClaim, rsEthIonPoolSF), "rsEthIonPool" ); assertLe( - expectedRswEthIonPoolClaim - rswEthIonPool.getUnderlyingClaimOf(address(vault)), + expectedRswEthIonPoolClaim - rswEthIonPool.balanceOf(address(vault)), postDepositClaimRE(expectedRswEthIonPoolClaim, rswEthIonPoolSF), "rswEthIonPool" ); @@ -1250,10 +1242,10 @@ abstract contract VaultWithIdlePool is VaultSharedSetup { uint256 prevTotalAssets = vault.totalAssets(); uint256 supplyFactor = rsEthIonPool.supplyFactor(); - uint256 weEthIonPoolClaim = weEthIonPool.getUnderlyingClaimOf(address(vault)); + uint256 weEthIonPoolClaim = weEthIonPool.balanceOf(address(vault)); uint256 idleClaim = BASE_ASSET.balanceOf(address(vault)); - uint256 rsEthIonPoolClaim = rsEthIonPool.getUnderlyingClaimOf(address(vault)); - uint256 rswEthIonPoolClaim = rswEthIonPool.getUnderlyingClaimOf(address(vault)); + uint256 rsEthIonPoolClaim = rsEthIonPool.balanceOf(address(vault)); + uint256 rswEthIonPoolClaim = rswEthIonPool.balanceOf(address(vault)); uint256 withdrawAmount = 40e18; @@ -1283,14 +1275,14 @@ abstract contract VaultWithIdlePool is VaultSharedSetup { postDepositClaimRE(withdrawAmount, supplyFactor), "vault total assets" ); - assertEq(weEthIonPool.getUnderlyingClaimOf(address(vault)), 0, "weEthIonPool claim"); + assertEq(weEthIonPool.balanceOf(address(vault)), 0, "weEthIonPool claim"); assertEq(BASE_ASSET.balanceOf(address(vault)), 0, "idle deposits"); assertLt( - (rsEthIonPoolClaim - rsEthIonPoolWithdraw) - rsEthIonPool.getUnderlyingClaimOf(address(vault)), + (rsEthIonPoolClaim - rsEthIonPoolWithdraw) - rsEthIonPool.balanceOf(address(vault)), postDepositClaimRE(withdrawAmount, supplyFactor), "rsEthIonPool claim" ); - assertEq(rswEthIonPool.getUnderlyingClaimOf(address(vault)), rswEthIonPoolClaim, "rswEthIonPool claim"); + assertEq(rswEthIonPool.balanceOf(address(vault)), rswEthIonPoolClaim, "rswEthIonPool claim"); // user gains withdrawn balance assertEq(BASE_ASSET.balanceOf(address(this)), withdrawAmount, "user base asset balance"); @@ -1349,10 +1341,10 @@ abstract contract VaultWithIdlePool is VaultSharedSetup { vault.updateAllocationCaps(ionPoolToUpdate, newAllocationCaps); // 10 weEth 20 idle 30 rsEth 10 rswEth - uint256 prevWeEthClaim = weEthIonPool.getUnderlyingClaimOf(address(vault)); + uint256 prevWeEthClaim = weEthIonPool.balanceOf(address(vault)); uint256 prevIdleClaim = BASE_ASSET.balanceOf(address(vault)); - uint256 prevRsEthClaim = rsEthIonPool.getUnderlyingClaimOf(address(vault)); - uint256 prevRswEthClaim = rswEthIonPool.getUnderlyingClaimOf(address(vault)); + uint256 prevRsEthClaim = rsEthIonPool.balanceOf(address(vault)); + uint256 prevRswEthClaim = rswEthIonPool.balanceOf(address(vault)); uint256 weEthSF = weEthIonPool.supplyFactor(); uint256 rsEthSF = rsEthIonPool.supplyFactor(); @@ -1375,17 +1367,13 @@ abstract contract VaultWithIdlePool is VaultSharedSetup { vault.reallocate(allocs); assertLt( - expectedWeEthClaim - weEthIonPool.getUnderlyingClaimOf(address(vault)), - postDepositClaimRE(0, weEthSF), - "weEthIonPol" + expectedWeEthClaim - weEthIonPool.balanceOf(address(vault)), postDepositClaimRE(0, weEthSF), "weEthIonPol" ); assertEq(BASE_ASSET.balanceOf(address(vault)), expectedIdleClaim, "IDLE"); assertLt( - expectedRsEthClaim - rsEthIonPool.getUnderlyingClaimOf(address(vault)), - postDepositClaimRE(0, rsEthSF), - "rswEthIonPol" + expectedRsEthClaim - rsEthIonPool.balanceOf(address(vault)), postDepositClaimRE(0, rsEthSF), "rswEthIonPol" ); - assertEq(prevRswEthClaim, rswEthIonPool.getUnderlyingClaimOf(address(vault)), "rswEthIonPool"); + assertEq(prevRswEthClaim, rswEthIonPool.balanceOf(address(vault)), "rswEthIonPool"); } function test_Reallocate_WithdrawFromIdle() public { @@ -1394,10 +1382,10 @@ abstract contract VaultWithIdlePool is VaultSharedSetup { vault.deposit(depositAmount, address(this)); // 10 weEth 20 idle 30 rsEth 10 rswEth - uint256 prevWeEthClaim = weEthIonPool.getUnderlyingClaimOf(address(vault)); + uint256 prevWeEthClaim = weEthIonPool.balanceOf(address(vault)); uint256 prevIdleClaim = BASE_ASSET.balanceOf(address(vault)); - uint256 prevRsEthClaim = rsEthIonPool.getUnderlyingClaimOf(address(vault)); - uint256 prevRswEthClaim = rswEthIonPool.getUnderlyingClaimOf(address(vault)); + uint256 prevRsEthClaim = rsEthIonPool.balanceOf(address(vault)); + uint256 prevRswEthClaim = rswEthIonPool.balanceOf(address(vault)); uint256 weEthSF = weEthIonPool.supplyFactor(); uint256 rswEthSF = rswEthIonPool.supplyFactor(); @@ -1420,17 +1408,15 @@ abstract contract VaultWithIdlePool is VaultSharedSetup { vault.reallocate(allocs); assertLt( - expectedWeEthClaim - weEthIonPool.getUnderlyingClaimOf(address(vault)), - postDepositClaimRE(0, weEthSF), - "weEthIonPol" + expectedWeEthClaim - weEthIonPool.balanceOf(address(vault)), postDepositClaimRE(0, weEthSF), "weEthIonPol" ); assertEq(BASE_ASSET.balanceOf(address(vault)), expectedIdleClaim, "IDLE"); assertLt( - expectedRswEthClaim - rswEthIonPool.getUnderlyingClaimOf(address(vault)), + expectedRswEthClaim - rswEthIonPool.balanceOf(address(vault)), postDepositClaimRE(0, rswEthSF), "rswEthIonPol" ); - assertEq(prevRsEthClaim, rsEthIonPool.getUnderlyingClaimOf(address(vault)), "rsEthIonPool"); + assertEq(prevRsEthClaim, rsEthIonPool.balanceOf(address(vault)), "rsEthIonPool"); } } diff --git a/test/unit/fuzz/IonPool.t.sol b/test/unit/fuzz/IonPool.t.sol index 57a82e6d..6414fe1b 100644 --- a/test/unit/fuzz/IonPool.t.sol +++ b/test/unit/fuzz/IonPool.t.sol @@ -71,11 +71,11 @@ abstract contract IonPool_LenderFuzzTestBase is IonPoolSharedSetup, IIonPoolEven ionPool.supply(lender1, supplyAmount, new bytes32[](0)); assertEq(lens.liquidity(iIonPool), supplyAmountBeforeSupply + supplyAmount, "weth"); - assertEq(ionPool.balanceOf(lender1), normalizedAmount, "ionPool balanceOf"); + assertEq(ionPool.normalizedBalanceOf(lender1), normalizedAmount, "ionPool balanceOf"); uint256 roundingError = currentSupplyFactor / RAY; - assertLe(ionPool.getUnderlyingClaimOf(lender1) - roundingError, supplyAmount, "balanceOf rounding error"); + assertLe(ionPool.balanceOf(lender1) - roundingError, supplyAmount, "balanceOf rounding error"); } function testFuzz_SupplyBaseToDifferentAddress(uint256 supplyAmount) public { @@ -105,10 +105,10 @@ abstract contract IonPool_LenderFuzzTestBase is IonPoolSharedSetup, IIonPoolEven ionPool.supply(address(this), supplyAmount, new bytes32[](0)); assertEq(lens.liquidity(iIonPool), supplyAmountBeforeSupply + supplyAmount); - assertEq(ionPool.balanceOf(address(this)), normalizedAmount); + assertEq(ionPool.normalizedBalanceOf(address(this)), normalizedAmount); uint256 roundingError = currentSupplyFactor / RAY; - assertLe(ionPool.getUnderlyingClaimOf(address(this)) - roundingError, supplyAmount); + assertLe(ionPool.balanceOf(address(this)) - roundingError, supplyAmount); } struct FuzzWithdrawBaseLocs { @@ -132,7 +132,7 @@ abstract contract IonPool_LenderFuzzTestBase is IonPoolSharedSetup, IIonPoolEven // Changing supply factor, means that the interest will be deposited _changeSupplyFactorIfNeeded(); uint256 supplyAmountAfterRebase = lens.liquidity(iIonPool); - uint256 lender1BalanceAfterRebase = ionPool.getUnderlyingClaimOf(lender1); + uint256 lender1BalanceAfterRebase = ionPool.balanceOf(lender1); assertEq(supplyAmountAfterRebase, lender1BalanceAfterRebase); @@ -141,7 +141,7 @@ abstract contract IonPool_LenderFuzzTestBase is IonPoolSharedSetup, IIonPoolEven vm.assume(locs.withdrawAmount > 0); uint256 underlyingBeforeWithdraw = underlying.balanceOf(lender1); - uint256 rewardAssetBalanceBeforeWithdraw = ionPool.getUnderlyingClaimOf(lender1); + uint256 rewardAssetBalanceBeforeWithdraw = ionPool.balanceOf(lender1); locs.currentTotalDebt = lens.debt(iIonPool); (locs.supplyFactorIncrease,,, locs.newDebtIncrease,) = ionPool.calculateRewardAndDebtDistribution(); @@ -159,7 +159,7 @@ abstract contract IonPool_LenderFuzzTestBase is IonPoolSharedSetup, IIonPoolEven ionPool.withdraw(lender1, locs.withdrawAmount); uint256 underlyingAfterWithdraw = underlying.balanceOf(lender1); - uint256 rewardAssetBalanceAfterWithdraw = ionPool.getUnderlyingClaimOf(lender1); + uint256 rewardAssetBalanceAfterWithdraw = ionPool.balanceOf(lender1); uint256 underlyingWithdrawn = underlyingAfterWithdraw - underlyingBeforeWithdraw; uint256 rewardAssetBurned = rewardAssetBalanceBeforeWithdraw - rewardAssetBalanceAfterWithdraw; @@ -170,7 +170,7 @@ abstract contract IonPool_LenderFuzzTestBase is IonPoolSharedSetup, IIonPoolEven assertGe(rewardAssetBurned, underlyingWithdrawn); uint256 roundingError = currentSupplyFactor / RAY; - assertLt(ionPool.balanceOf(lender1), lender1BalanceAfterRebase - locs.withdrawAmount + roundingError); + assertLt(ionPool.normalizedBalanceOf(lender1), lender1BalanceAfterRebase - locs.withdrawAmount + roundingError); } function testFuzz_WithdrawBaseToDifferentAddress(uint256 supplyAmount, uint256 withdrawAmount) public { @@ -187,7 +187,7 @@ abstract contract IonPool_LenderFuzzTestBase is IonPoolSharedSetup, IIonPoolEven // Changing supply factor, means that the interest will be deposited _changeSupplyFactorIfNeeded(); uint256 supplyAmountAfterRebase = lens.liquidity(iIonPool); - uint256 lender1BalanceAfterRebase = ionPool.getUnderlyingClaimOf(lender1); + uint256 lender1BalanceAfterRebase = ionPool.balanceOf(lender1); assertEq(supplyAmountAfterRebase, lender1BalanceAfterRebase, "amount after rebase"); @@ -196,7 +196,7 @@ abstract contract IonPool_LenderFuzzTestBase is IonPoolSharedSetup, IIonPoolEven vm.assume(locs.withdrawAmount > 0); uint256 underlyingBeforeWithdraw = underlying.balanceOf(lender2); - uint256 rewardAssetBalanceBeforeWithdraw = ionPool.getUnderlyingClaimOf(lender1); + uint256 rewardAssetBalanceBeforeWithdraw = ionPool.balanceOf(lender1); locs.currentTotalDebt = lens.debt(iIonPool); (locs.supplyFactorIncrease,,, locs.newDebtIncrease,) = ionPool.calculateRewardAndDebtDistribution(); @@ -214,7 +214,7 @@ abstract contract IonPool_LenderFuzzTestBase is IonPoolSharedSetup, IIonPoolEven ionPool.withdraw(lender2, locs.withdrawAmount); uint256 underlyingAfterWithdraw = underlying.balanceOf(lender2); - uint256 rewardAssetBalanceAfterWithdraw = ionPool.getUnderlyingClaimOf(lender1); + uint256 rewardAssetBalanceAfterWithdraw = ionPool.balanceOf(lender1); uint256 underlyingWithdrawn = underlyingAfterWithdraw - underlyingBeforeWithdraw; uint256 rewardAssetBurned = rewardAssetBalanceBeforeWithdraw - rewardAssetBalanceAfterWithdraw; @@ -226,7 +226,7 @@ abstract contract IonPool_LenderFuzzTestBase is IonPoolSharedSetup, IIonPoolEven uint256 roundingError = currentSupplyFactor / RAY; assertLt( - ionPool.getUnderlyingClaimOf(lender1), + ionPool.balanceOf(lender1), lender1BalanceAfterRebase - locs.withdrawAmount + roundingError, "underlying claim" ); @@ -1268,7 +1268,7 @@ contract IonPool_BorrowerFuzzTest is IonPool_BorrowerFuzzTestBase { assertEq(lens.liquidity(iIonPool), INITIAL_LENDER_UNDERLYING_BALANCE); assertEq(underlying.balanceOf(address(ionPool)), INITIAL_LENDER_UNDERLYING_BALANCE); - assertEq(ionPool.balanceOf(lender2), INITIAL_LENDER_UNDERLYING_BALANCE); + assertEq(ionPool.normalizedBalanceOf(lender2), INITIAL_LENDER_UNDERLYING_BALANCE); for (uint256 i = 0; i < lens.ilkCount(iIonPool); i++) { assertEq(collaterals[i].allowance(borrower1, address(gemJoins[i])), type(uint256).max); diff --git a/test/unit/fuzz/RewardToken.t.sol b/test/unit/fuzz/RewardToken.t.sol index 6ffbfa3a..e8ad4d6b 100644 --- a/test/unit/fuzz/RewardToken.t.sol +++ b/test/unit/fuzz/RewardToken.t.sol @@ -29,7 +29,7 @@ contract RewardToken_FuzzUnitTest is RewardTokenSharedSetup { rewardModule.mint(address(0), amountOfRewards); rewardModule.mint(address(this), amountOfRewards); - assertEq(rewardModule.balanceOf(address(this)), amountOfRewards); + assertEq(rewardModule.normalizedBalanceOf(address(this)), amountOfRewards); assertEq(underlying.balanceOf(address(this)), 0); assertEq(underlying.balanceOf(address(rewardModule)), amountOfRewards); } @@ -45,14 +45,14 @@ contract RewardToken_FuzzUnitTest is RewardTokenSharedSetup { underlying.approve(address(rewardModule), amountOfRewards); rewardModule.mint(address(this), amountOfRewards); - assertEq(rewardModule.balanceOf(address(this)), amountOfRewards); + assertEq(rewardModule.normalizedBalanceOf(address(this)), amountOfRewards); assertEq(underlying.balanceOf(address(this)), 0); vm.expectRevert(abi.encodeWithSelector(RewardToken.InvalidSender.selector, address(0))); rewardModule.burn(address(0), address(this), amountOfRewards); rewardModule.burn(address(this), address(this), amountOfRewards); - assertEq(rewardModule.balanceOf(address(this)), 0); + assertEq(rewardModule.normalizedBalanceOf(address(this)), 0); } function testFuzz_MintRewardWithSupplyFactorChange(uint256 amountOfRewards, uint256 supplyFactorNew) external { @@ -71,8 +71,8 @@ contract RewardToken_FuzzUnitTest is RewardTokenSharedSetup { uint256 expectedNormalizedMint1 = amountOfRewards.rayDivDown(supplyFactorOld); - assertEq(rewardModule.balanceOf(address(this)), expectedNormalizedMint1); - assertEq(rewardModule.getUnderlyingClaimOf(address(this)), amountOfRewards); + assertEq(rewardModule.normalizedBalanceOf(address(this)), expectedNormalizedMint1); + assertEq(rewardModule.balanceOf(address(this)), amountOfRewards); assertEq(underlying.balanceOf(address(this)), 0); assertEq(underlying.balanceOf(address(rewardModule)), amountOfRewards); @@ -90,8 +90,8 @@ contract RewardToken_FuzzUnitTest is RewardTokenSharedSetup { uint256 totalDepositsNormalized = expectedNormalizedMint1 + expectedNormalizedMint2; uint256 totalValue = totalDepositsNormalized.rayMulDown(supplyFactorNew); - assertEq(rewardModule.balanceOf(address(this)), totalDepositsNormalized); - assertEq(rewardModule.getUnderlyingClaimOf(address(this)), totalValue); + assertEq(rewardModule.normalizedBalanceOf(address(this)), totalDepositsNormalized); + assertEq(rewardModule.balanceOf(address(this)), totalValue); assertEq(underlying.balanceOf(address(this)), 0); assertEq(underlying.balanceOf(address(rewardModule)), totalDeposited + interestCreated); } @@ -112,8 +112,8 @@ contract RewardToken_FuzzUnitTest is RewardTokenSharedSetup { uint256 expectedNormalizedMint1 = amountOfRewards.rayDivDown(supplyFactorOld); - assertEq(rewardModule.balanceOf(address(this)), expectedNormalizedMint1); - assertEq(rewardModule.getUnderlyingClaimOf(address(this)), amountOfRewards); + assertEq(rewardModule.normalizedBalanceOf(address(this)), expectedNormalizedMint1); + assertEq(rewardModule.balanceOf(address(this)), amountOfRewards); assertEq(underlying.balanceOf(address(this)), 0); assertEq(underlying.balanceOf(address(rewardModule)), amountOfRewards); @@ -131,8 +131,8 @@ contract RewardToken_FuzzUnitTest is RewardTokenSharedSetup { uint256 totalDepositsNormalized = expectedNormalizedMint1 + expectedNormalizedMint2; uint256 totalValue = totalDepositsNormalized.rayMulDown(supplyFactorNew); - assertEq(rewardModule.balanceOf(address(this)), totalDepositsNormalized); - assertEq(rewardModule.getUnderlyingClaimOf(address(this)), totalValue); + assertEq(rewardModule.normalizedBalanceOf(address(this)), totalDepositsNormalized); + assertEq(rewardModule.balanceOf(address(this)), totalValue); assertEq(underlying.balanceOf(address(this)), 0); assertEq(underlying.balanceOf(address(rewardModule)), totalDeposited + interestCreated); @@ -153,7 +153,7 @@ contract RewardToken_FuzzUnitTest is RewardTokenSharedSetup { underlying.approve(address(rewardModule), amountOfRewardTokens); rewardModule.mint(address(this), amountOfRewardTokens); - assertEq(rewardModule.balanceOf(address(this)), amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(address(this)), amountOfRewardTokens); assertEq(underlying.balanceOf(address(this)), 0); assertEq(underlying.balanceOf(address(rewardModule)), amountOfRewardTokens); @@ -172,8 +172,8 @@ contract RewardToken_FuzzUnitTest is RewardTokenSharedSetup { rewardModule.transfer(address(this), amountOfRewardTokens); rewardModule.transfer(receivingUser, amountOfRewardTokens); - assertEq(rewardModule.balanceOf(address(this)), 0); - assertEq(rewardModule.balanceOf(receivingUser), amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(address(this)), 0); + assertEq(rewardModule.normalizedBalanceOf(receivingUser), amountOfRewardTokens); assertEq(underlying.balanceOf(address(this)), 0); } @@ -186,8 +186,8 @@ contract RewardToken_FuzzUnitTest is RewardTokenSharedSetup { underlying.approve(address(rewardModule), amountOfRewardTokens); rewardModule.mint(sendingUser, amountOfRewardTokens); - assertEq(rewardModule.balanceOf(sendingUser), amountOfRewardTokens); - assertEq(rewardModule.balanceOf(receivingUser), 0); + assertEq(rewardModule.normalizedBalanceOf(sendingUser), amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(receivingUser), 0); assertEq(rewardModule.allowance(sendingUser, spender), 0); assertEq(underlying.balanceOf(address(this)), 0); assertEq(underlying.balanceOf(address(rewardModule)), amountOfRewardTokens); @@ -215,8 +215,8 @@ contract RewardToken_FuzzUnitTest is RewardTokenSharedSetup { rewardModule.transferFrom(sendingUser, receivingUser, amountOfRewardTokens + 1); rewardModule.transferFrom(sendingUser, receivingUser, amountOfRewardTokens); - assertEq(rewardModule.balanceOf(sendingUser), 0); - assertEq(rewardModule.balanceOf(receivingUser), amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(sendingUser), 0); + assertEq(rewardModule.normalizedBalanceOf(receivingUser), amountOfRewardTokens); assertEq(rewardModule.allowance(sendingUser, spender), 0); } @@ -229,8 +229,8 @@ contract RewardToken_FuzzUnitTest is RewardTokenSharedSetup { underlying.approve(address(rewardModule), amountOfRewardTokens); rewardModule.mint(sendingUser, amountOfRewardTokens); - assertEq(rewardModule.balanceOf(sendingUser), amountOfRewardTokens); - assertEq(rewardModule.balanceOf(receivingUser), 0); + assertEq(rewardModule.normalizedBalanceOf(sendingUser), amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(receivingUser), 0); assertEq(rewardModule.allowance(sendingUser, spender), 0); assertEq(underlying.balanceOf(address(this)), 0); assertEq(underlying.balanceOf(address(rewardModule)), amountOfRewardTokens); @@ -278,8 +278,8 @@ contract RewardToken_FuzzUnitTest is RewardTokenSharedSetup { rewardModule.transferFrom(sendingUser, receivingUser, amountOfRewardTokens + 1); rewardModule.transferFrom(sendingUser, receivingUser, amountOfRewardTokens); - assertEq(rewardModule.balanceOf(sendingUser), 0); - assertEq(rewardModule.balanceOf(receivingUser), amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(sendingUser), 0); + assertEq(rewardModule.normalizedBalanceOf(receivingUser), amountOfRewardTokens); assertEq(rewardModule.allowance(sendingUser, spender), 0); } @@ -306,8 +306,8 @@ contract RewardToken_FuzzUnitTest is RewardTokenSharedSetup { underlying.approve(address(rewardModule), locals.amountOfRewardTokens); rewardModule.mint(sendingUser, locals.amountOfRewardTokens); - assertEq(rewardModule.balanceOf(sendingUser), locals.amountOfRewardTokens); - assertEq(rewardModule.balanceOf(receivingUser), 0); + assertEq(rewardModule.normalizedBalanceOf(sendingUser), locals.amountOfRewardTokens); + assertEq(rewardModule.normalizedBalanceOf(receivingUser), 0); assertEq(rewardModule.allowance(sendingUser, spender), 0); assertEq(underlying.balanceOf(address(this)), 0); assertEq(underlying.balanceOf(address(rewardModule)), locals.amountOfRewardTokens); diff --git a/test/unit/fuzz/vault/Vault.t.sol b/test/unit/fuzz/vault/Vault.t.sol index 5c8011ef..7f034cc1 100644 --- a/test/unit/fuzz/vault/Vault.t.sol +++ b/test/unit/fuzz/vault/Vault.t.sol @@ -37,7 +37,7 @@ contract Vault_Fuzz is VaultSharedSetup { weEthIonPool.supply(address(this), assets, new bytes32[](0)); uint256 expectedClaim = assets; - uint256 resultingClaim = weEthIonPool.getUnderlyingClaimOf(address(this)); + uint256 resultingClaim = weEthIonPool.balanceOf(address(this)); uint256 re = assets - ((assets * RAY - ((assets * RAY) % supplyFactor)) / RAY); assertLe(expectedClaim - resultingClaim, (supplyFactor - 2) / RAY + 1); From c7bf309e9dba7e19eb81e456fa4cd3254e5e65b9 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Mon, 6 May 2024 21:38:58 -0400 Subject: [PATCH 02/20] fix: querying unaccrued interest when paused, caching IDLE balance in reallocate, accrue fee when feePercentage is updated --- src/interfaces/IIonPool.sol | 1 + src/token/RewardToken.sol | 16 ++- src/vault/Vault.sol | 22 ++- test/unit/concrete/vault/Vault.t.sol | 198 ++++++++++++++++++++++++++- test/unit/fuzz/vault/Vault.t.sol | 111 +++++++++++++++ 5 files changed, 337 insertions(+), 11 deletions(-) diff --git a/src/interfaces/IIonPool.sol b/src/interfaces/IIonPool.sol index b28d071f..b7b9e587 100644 --- a/src/interfaces/IIonPool.sol +++ b/src/interfaces/IIonPool.sol @@ -234,4 +234,5 @@ interface IIonPool { function getTotalUnderlyingClaims() external view returns (uint256); function getUnderlyingClaimOf(address user) external view returns (uint256); function extsload(bytes32 slot) external view returns (bytes32); + function balanceOfUnaccrued(address user) external view returns (uint256); } diff --git a/src/token/RewardToken.sol b/src/token/RewardToken.sol index d50aae14..c1887128 100644 --- a/src/token/RewardToken.sol +++ b/src/token/RewardToken.sol @@ -333,8 +333,6 @@ abstract contract RewardToken is $._normalizedBalances[from] = oldSenderBalance - amountNormalized; } $._normalizedBalances[to] += amountNormalized; - - emit Transfer(from, to, amountNormalized); } /** @@ -450,7 +448,7 @@ abstract contract RewardToken is } /** - * @dev Current token balance + * @dev Current claim of the underlying token inclusive of interest to be accrued. * @param user to get balance of */ function balanceOf(address user) public view returns (uint256) { @@ -461,6 +459,14 @@ abstract contract RewardToken is return $._normalizedBalances[user].rayMulDown($.supplyFactor + totalSupplyFactorIncrease); } + /** + * @dev Current claim of the underlying token without accounting for interest to be accrued. + */ + function balanceOfUnaccrued(address user) public view returns (uint256) { + RewardTokenStorage storage $ = _getRewardTokenStorage(); + return $._normalizedBalances[user].rayMulDown($.supplyFactor); + } + /** * @dev Accounting is done in normalized balances * @param user to get normalized balance of @@ -532,9 +538,7 @@ abstract contract RewardToken is } /** - * @dev Current total supply - * - * Normalized total supply and total supply are same in non-rebasing token. + * @dev Normalized total supply. */ function normalizedTotalSupply() public view returns (uint256) { RewardTokenStorage storage $ = _getRewardTokenStorage(); diff --git a/src/vault/Vault.sol b/src/vault/Vault.sol index 33d19436..a3569fcd 100644 --- a/src/vault/Vault.sol +++ b/src/vault/Vault.sol @@ -16,7 +16,6 @@ import { Multicall } from "@openzeppelin/contracts/utils/Multicall.sol"; import { ReentrancyGuard } from "openzeppelin-contracts/contracts/utils/ReentrancyGuard.sol"; import { AccessControlDefaultAdminRules } from "@openzeppelin/contracts/access/extensions/AccessControlDefaultAdminRules.sol"; - /** * @title Ion Lending Vault * @author Molecular Labs @@ -28,6 +27,7 @@ import { AccessControlDefaultAdminRules } from * * @custom:security-contact security@molecularlabs.io */ + contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, ReentrancyGuard { using EnumerableSet for EnumerableSet.AddressSet; using Math for uint256; @@ -115,6 +115,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy */ function updateFeePercentage(uint256 _feePercentage) external onlyRole(OWNER_ROLE) { if (_feePercentage > RAY) revert InvalidFeePercentage(); + _accrueFee(); feePercentage = _feePercentage; } @@ -343,6 +344,8 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy // to the user from the previous function scope. if (pool != IDLE) { pool.withdraw(address(this), transferAmt); + } else { + currentIdleDeposits -= transferAmt; } totalWithdrawn += transferAmt; @@ -372,6 +375,8 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy // contract. if (pool != IDLE) { pool.supply(address(this), transferAmt, new bytes32[](0)); + } else { + currentIdleDeposits += transferAmt; } totalSupplied += transferAmt; @@ -655,7 +660,16 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy for (uint256 i; i != _supportedMarketsLength;) { IIonPool pool = IIonPool(supportedMarkets.at(i)); - uint256 assetsInPool = pool == IDLE ? BASE_ASSET.balanceOf(address(this)) : pool.balanceOf(address(this)); + uint256 assetsInPool; + if (pool == IDLE) { + assetsInPool = BASE_ASSET.balanceOf(address(this)); + } else { + if (pool.paused()) { + assetsInPool = pool.balanceOfUnaccrued(address(this)); + } else { + assetsInPool = pool.balanceOf(address(this)); + } + } assets += assetsInPool; @@ -762,7 +776,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy (feeShares, newTotalAssets) = _accruedFeeShares(); if (feeShares != 0) _mint(feeRecipient, feeShares); - lastTotalAssets = newTotalAssets; // This update happens outside of this function in Metamorpho. + lastTotalAssets = newTotalAssets; emit FeeAccrued(feeShares, newTotalAssets); } @@ -896,7 +910,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy * @return The max amount of assets withdrawable from this IonPool. */ function _withdrawable(IIonPool pool) internal view returns (uint256) { - uint256 currentSupplied = pool.balanceOf(address(this)); // TODO should be balanceOf + uint256 currentSupplied = pool.balanceOf(address(this)); uint256 availableLiquidity = uint256(pool.extsload(ION_POOL_LIQUIDITY_SLOT)); return Math.min(currentSupplied, availableLiquidity); diff --git a/test/unit/concrete/vault/Vault.t.sol b/test/unit/concrete/vault/Vault.t.sol index bb73be56..60c3168a 100644 --- a/test/unit/concrete/vault/Vault.t.sol +++ b/test/unit/concrete/vault/Vault.t.sol @@ -1423,7 +1423,109 @@ abstract contract VaultWithIdlePool is VaultSharedSetup { contract VaultERC4626ExternalViews is VaultSharedSetup { function setUp() public override { super.setUp(); - // markets.push(IDLE); + } + + function test_TotalAssetsWithSinglePausedIonPool() public { + weEthIonPool.updateSupplyCap(type(uint256).max); + weEthIonPool.updateIlkDebtCeiling(0, type(uint256).max); + + supply(address(this), weEthIonPool, 1000e18); + borrow(address(this), weEthIonPool, weEthGemJoin, 100e18, 70e18); + + uint256[] memory allocationCaps = new uint256[](3); + allocationCaps[0] = 20e18; + allocationCaps[1] = 0; + allocationCaps[2] = 0; + + vm.prank(OWNER); + vault.updateAllocationCaps(markets, allocationCaps); + + uint256 depositAmt = 10e18; + setERC20Balance(address(BASE_ASSET), address(this), depositAmt); + vault.deposit(depositAmt, address(this)); + + assertEq(weEthIonPool.balanceOf(address(vault)), depositAmt, "weEthIonPool balance"); + // pause the weEthIonPool + weEthIonPool.pause(); + + vm.warp(block.timestamp + 365 days); + + assertGt(weEthIonPool.balanceOf(address(vault)), depositAmt, "weEthIonPool accrues interest"); + assertLt( + weEthIonPool.balanceOfUnaccrued(address(vault)), + weEthIonPool.balanceOf(address(vault)), + "weEthIonPool unaccrued balance" + ); + assertTrue(weEthIonPool.paused(), "weEthIonPool is paused"); + + uint256 totalAssets = vault.totalAssets(); + assertEq(totalAssets, depositAmt, "total assets with paused IonPool does not include interest"); + } + + function test_TotalAssetsWithMultiplePausedIonPools() public { + // Make sure every pool has debt to accrue interest from + uint256 initialSupplyAmt = 1000e18; + weEthIonPool.updateSupplyCap(type(uint256).max); + rsEthIonPool.updateSupplyCap(type(uint256).max); + rswEthIonPool.updateSupplyCap(type(uint256).max); + + weEthIonPool.updateIlkDebtCeiling(0, type(uint256).max); + rsEthIonPool.updateIlkDebtCeiling(0, type(uint256).max); + rswEthIonPool.updateIlkDebtCeiling(0, type(uint256).max); + + supply(address(this), weEthIonPool, initialSupplyAmt); + borrow(address(this), weEthIonPool, weEthGemJoin, 100e18, 70e18); + + supply(address(this), rsEthIonPool, initialSupplyAmt); + borrow(address(this), rsEthIonPool, rsEthGemJoin, 100e18, 70e18); + + supply(address(this), rswEthIonPool, initialSupplyAmt); + borrow(address(this), rswEthIonPool, rswEthGemJoin, 100e18, 70e18); + + uint256[] memory allocationCaps = new uint256[](3); + uint256 weEthIonPoolAmt = 10e18; + uint256 rsEthIonPoolAmt = 20e18; + uint256 rswEthIonPoolAmt = 30e18; + allocationCaps[0] = weEthIonPoolAmt; + allocationCaps[1] = rsEthIonPoolAmt; + allocationCaps[2] = rswEthIonPoolAmt; + + vm.prank(OWNER); + vault.updateAllocationCaps(markets, allocationCaps); + + uint256 depositAmt = 60e18; + setERC20Balance(address(BASE_ASSET), address(this), depositAmt); + vault.deposit(depositAmt, address(this)); + + assertEq(weEthIonPool.balanceOf(address(vault)), weEthIonPoolAmt, "weEthIonPool balance"); + assertEq(rsEthIonPool.balanceOf(address(vault)), rsEthIonPoolAmt, "rsEthIonPool balance"); + assertEq(rswEthIonPool.balanceOf(address(vault)), rswEthIonPoolAmt, "rswEthIonPool balance"); + + weEthIonPool.pause(); + // NOTE rsEthIonPool is not paused + rswEthIonPool.pause(); + + assertTrue(weEthIonPool.paused(), "weEthIonPool is paused"); + assertFalse(rsEthIonPool.paused(), "rsEthIonPool is not paused"); + assertTrue(rswEthIonPool.paused(), "rswEthIonPool is paused"); + + vm.warp(block.timestamp + 365 days); + + assertGt(weEthIonPool.balanceOf(address(vault)), weEthIonPoolAmt, "weEthIonPool balance increases"); + assertGt(rsEthIonPool.balanceOf(address(vault)), rsEthIonPoolAmt, "rsEthIonPool balance does not change"); + assertGt(rswEthIonPool.balanceOf(address(vault)), rswEthIonPoolAmt, "rswEthIonPool balance increases"); + + // The 'unaccrued' values should not change + assertEq(weEthIonPool.balanceOfUnaccrued(address(vault)), weEthIonPoolAmt, "weEthIonPool balance"); + assertEq(rsEthIonPool.balanceOfUnaccrued(address(vault)), rsEthIonPoolAmt, "rsEthIonPool balance"); + assertEq(rswEthIonPool.balanceOfUnaccrued(address(vault)), rswEthIonPoolAmt, "rswEthIonPool balance"); + + uint256 expectedTotalAssets = weEthIonPool.balanceOfUnaccrued(address(vault)) + + rsEthIonPool.balanceOf(address(vault)) + rswEthIonPool.balanceOfUnaccrued(address(vault)); + + assertEq( + vault.totalAssets(), expectedTotalAssets, "total assets without accounting for interest in paused IonPools" + ); } // --- Max --- @@ -1495,6 +1597,100 @@ contract VaultERC4626ExternalViews is VaultSharedSetup { function test_PreviewRedeem() public { } } +contract VaultInflationAttack is VaultSharedSetup { + function setUp() public override { + super.setUp(); + } + + /** + * Starting Attacker Balance: 11e18 + 10 + * Attacker Mint: 10 shares + * Attacker Donation: 11e18 + * Alice Deposit: 1e18 + * Alice Shares Minted: + * + * How much did the attacker lose during the donation? + * Attacker Donated 11e18, + */ + function test_InflationAttackNotProfitable() public { + IIonPool[] memory market = new IIonPool[](1); + market[0] = IDLE; + + uint256[] memory allocationCaps = new uint256[](1); + allocationCaps[0] = type(uint256).max; + + IIonPool[] memory queue = new IIonPool[](4); + queue[0] = IDLE; + queue[1] = weEthIonPool; + queue[2] = rsEthIonPool; + queue[3] = rswEthIonPool; + + vm.prank(OWNER); + vault.addSupportedMarkets(market, allocationCaps, queue, queue); + + uint256 donationAmt = 11e18; + uint256 mintAmt = 10; + + // fund attacker + setERC20Balance(address(BASE_ASSET), address(this), donationAmt + mintAmt); + + uint256 initialAssetBalance = BASE_ASSET.balanceOf(address(this)); + console2.log("attacker balance before : "); + console2.log(initialAssetBalance); + + vault.mint(mintAmt, address(this)); + uint256 attackerClaimAfterMint = vault.previewRedeem(vault.balanceOf(address(this))); + + console2.log("attackerClaimAfterMint: "); + console2.log(attackerClaimAfterMint); + + console2.log("donationAmt: "); + console2.log(donationAmt); + + // donate to inflate exchange rate by increasing `totalAssets` + IERC20(address(BASE_ASSET)).transfer(address(vault), donationAmt); + + // how much of this donation was captured by the virtual shares on the vault? + uint256 attackerClaimAfterDonation = vault.previewRedeem(vault.balanceOf(address(this))); + + console2.log("attackerClaimAfterDonation: "); + console2.log(attackerClaimAfterDonation); + + uint256 lossFromDonation = attackerClaimAfterMint + donationAmt - attackerClaimAfterDonation; + + console2.log("loss from donation: "); + console2.log(lossFromDonation); + + address alice = address(0xabcd); + setERC20Balance(address(BASE_ASSET), alice, 10e18 + 10); + + vm.startPrank(alice); + IERC20(address(BASE_ASSET)).approve(address(vault), 1e18); + vault.deposit(1e18, alice); + vm.stopPrank(); + + // Alice gained zero shares due to exchange rate inflation + uint256 aliceShares = vault.balanceOf(alice); + console.log("alice must lose all her shares : "); + console.log(aliceShares); + + // How much of alice's deposits were captured by the attacker's shares? + uint256 attackerClaimAfterAlice = vault.previewRedeem(vault.balanceOf(address(this))); + uint256 attackerGainFromAlice = attackerClaimAfterAlice - attackerClaimAfterDonation; + console2.log("attackerGainFromAlice: "); + console2.log(attackerGainFromAlice); + + vault.redeem(vault.balanceOf(address(this)) - 3, address(this), address(this)); + uint256 afterAssetBalance = BASE_ASSET.balanceOf(address(this)); + + console.log("attacker balance after : "); + console.log(afterAssetBalance); + + assertLe(attackerGainFromAlice, lossFromDonation, "attack must not be profitable"); + assertLe(afterAssetBalance, initialAssetBalance, "attacker must not be profitable"); + } +} + contract VaultDeposit_WithoutSupplyFactor is VaultDeposit { function setUp() public override(VaultDeposit) { super.setUp(); diff --git a/test/unit/fuzz/vault/Vault.t.sol b/test/unit/fuzz/vault/Vault.t.sol index 7f034cc1..32cc524f 100644 --- a/test/unit/fuzz/vault/Vault.t.sol +++ b/test/unit/fuzz/vault/Vault.t.sol @@ -1,11 +1,13 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.21; +import { IIonPool } from "./../../../../src/interfaces/IIonPool.sol"; import { IonPoolExposed } from "../../../helpers/IonPoolSharedSetup.sol"; import { VaultSharedSetup } from "../../../helpers/VaultSharedSetup.sol"; import { Math } from "openzeppelin-contracts/contracts/utils/math/Math.sol"; import { WadRayMath, RAY, WAD } from "./../../../../src/libraries/math/WadRayMath.sol"; import { console2 } from "forge-std/console2.sol"; +import { IERC20 } from "openzeppelin-contracts/contracts/interfaces/IERC20.sol"; using Math for uint256; @@ -42,6 +44,17 @@ contract Vault_Fuzz is VaultSharedSetup { uint256 re = assets - ((assets * RAY - ((assets * RAY) % supplyFactor)) / RAY); assertLe(expectedClaim - resultingClaim, (supplyFactor - 2) / RAY + 1); } + + function testFuzz_FullyWithdrawableFromIonPool(uint256 assets) public { + uint256 supplyFactor = bound(assets, 1e27, 10e45); + uint256 normalizedAmt = bound(assets, 0, type(uint48).max); + + uint256 claim = normalizedAmt * supplyFactor / RAY; + uint256 sharesToBurn = claim * RAY / supplyFactor; + sharesToBurn = sharesToBurn * supplyFactor < claim * RAY ? sharesToBurn + 1 : sharesToBurn; + + assertEq(normalizedAmt, sharesToBurn); + } } contract VaultWithYieldAndFee_Fuzz is VaultSharedSetup { @@ -158,3 +171,101 @@ contract VaultWithYieldAndFee_Fuzz is VaultSharedSetup { function testFuzz_MaxDepositAndMaxMint(uint256 assets) public { } } + +contract VaultInflationAttack is VaultSharedSetup { + address immutable ATTACKER = newAddress("attacker"); + address immutable USER = newAddress("user"); + + function setUp() public override { + super.setUp(); + + IIonPool[] memory market = new IIonPool[](1); + market[0] = IDLE; + + uint256[] memory allocationCaps = new uint256[](1); + allocationCaps[0] = type(uint256).max; + + IIonPool[] memory queue = new IIonPool[](4); + queue[0] = IDLE; + queue[1] = weEthIonPool; + queue[2] = rsEthIonPool; + queue[3] = rswEthIonPool; + + vm.prank(OWNER); + vault.addSupportedMarkets(market, allocationCaps, queue, queue); + + vm.prank(ATTACKER); + IERC20(address(BASE_ASSET)).approve(address(vault), type(uint256).max); + vm.prank(USER); + IERC20(address(BASE_ASSET)).approve(address(vault), type(uint256).max); + } + + function testFuzz_InflationAttackNotProfitable(uint256 assets) public { + // 1. The vault has not been used. + // - no shares minted and no assets deposited. + // - but the initial conversion is dictated by virtual shares. + assertEq(vault.totalSupply(), 0, "initial total supply"); + assertEq(vault.totalAssets(), 0, "initial total assets"); + + // 2. The attacker makes a first deposit. + uint256 firstDepositAmt = bound(assets, 0, type(uint128).max); + setERC20Balance(address(BASE_ASSET), ATTACKER, firstDepositAmt); + + vm.prank(ATTACKER); + vault.mint(firstDepositAmt, ATTACKER); + + uint256 attackerClaimAfterMint = vault.previewRedeem(vault.balanceOf(ATTACKER)); + + // check that the mint amount and transfer amount was the same + assertEq(BASE_ASSET.balanceOf(ATTACKER), 0, "mint amount equals transfer amount"); + + // 3. The attacker donates. + // - In this case, transfers to vault to increase IDLE deposits. + // - Check that the attacker loses a portion of the donated funds. + uint256 donationAmt = bound(assets, 0, type(uint128).max); + setERC20Balance(address(BASE_ASSET), ATTACKER, donationAmt); + + vm.prank(ATTACKER); + IERC20(address(BASE_ASSET)).transfer(address(vault), donationAmt); + + uint256 attackerClaimAfterDonation = vault.previewRedeem(vault.balanceOf(ATTACKER)); + uint256 attackerLossFromDonation = donationAmt - (attackerClaimAfterDonation - attackerClaimAfterMint); + + uint256 totalAssetsBeforeDeposit = vault.totalAssets(); + uint256 totalSupplyBeforeDeposit = vault.totalSupply(); + + // 4. A user makes a deposit where the shares truncate to zero. + // - sharesToMint = depositAmt * (newTotalSupply + 1) / (newTotalAssets + 1) + // - The sharesToMint must be less than 1 to round down to zero + // - depositAmt * (newTotalSupply + 1) / (newTotalAssets + 1) < 1 + // - depositAmt < 1 * (newTotalAssets + 1) / (newTotalSupply + 1) + uint256 maxDepositAmt = (vault.totalAssets() + 1) / (vault.totalSupply() + 1); + uint256 userDepositAmt = bound(assets, 0, maxDepositAmt); + + vm.startPrank(USER); + setERC20Balance(address(BASE_ASSET), USER, userDepositAmt); + IERC20(address(BASE_ASSET)).approve(address(vault), userDepositAmt); + vault.deposit(userDepositAmt, USER); + vm.stopPrank(); + + assertEq(vault.balanceOf(USER), 0, "user minted shares must be zero"); + + uint256 attackerClaimAfterUser = vault.previewRedeem(vault.balanceOf(ATTACKER)); + uint256 attackerGainFromUser = attackerClaimAfterUser - attackerClaimAfterDonation; + + // loss = donationAmt / (1 + firstDepositAmt) + uint256 expectedAttackerLossFromDonation = donationAmt / (1 + firstDepositAmt); + assertLe( + attackerLossFromDonation - expectedAttackerLossFromDonation, + 1, + "attacker loss from donation as expected with rounding error" + ); + + // INVARIANT: The money gained from the user must be less than or equal to the attacker's loss from the + // donation. + // assertLe(attackerGainFromUser, attackerLossFromDonation, "attacker must not profit from user"); + assertLe(userDepositAmt, attackerLossFromDonation, "loss must be ge to user deposit"); + } + + function testFuzz_InflationAttackSmallerDegree() public { } +} From bc3ad910aef3d7ae9686dd054c13ec5759c803a6 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Sat, 11 May 2024 14:19:54 -0400 Subject: [PATCH 03/20] fix: add max number of supported markets allowed --- src/vault/Vault.sol | 5 +++++ test/unit/concrete/vault/Vault.t.sol | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/vault/Vault.sol b/src/vault/Vault.sol index fc3231db..6a9509de 100644 --- a/src/vault/Vault.sol +++ b/src/vault/Vault.sol @@ -46,6 +46,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy error MarketsAndAllocationCapLengthMustBeEqual(); error IonPoolsArrayAndNewCapsArrayMustBeOfEqualLength(); error InvalidFeePercentage(); + error MaxSupportedMarketsReached(); event UpdateSupplyQueue(address indexed caller, IIonPool[] newSupplyQueue); event UpdateWithdrawQueue(address indexed caller, IIonPool[] newWithdrawQueue); @@ -69,6 +70,8 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy IERC20 public immutable BASE_ASSET; + uint8 public constant MAX_SUPPORTED_MARKETS = 32; + EnumerableSet.AddressSet supportedMarkets; IIonPool[] public supplyQueue; @@ -167,6 +170,8 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy } } + if (supportedMarkets.length() > MAX_SUPPORTED_MARKETS) revert MaxSupportedMarketsReached(); + updateSupplyQueue(newSupplyQueue); updateWithdrawQueue(newWithdrawQueue); } diff --git a/test/unit/concrete/vault/Vault.t.sol b/test/unit/concrete/vault/Vault.t.sol index cafcdb22..aa68a09c 100644 --- a/test/unit/concrete/vault/Vault.t.sol +++ b/test/unit/concrete/vault/Vault.t.sol @@ -162,6 +162,28 @@ contract VaultSetUpTest is VaultSharedSetup { function test_Revert_AddSupportedMarkets_MarketAlreadySupported() public { } + function test_Revert_AddSupportedMarkets_MaxSupportedMarketsReached() public { + vault = new Vault(BASE_ASSET, FEE_RECIPIENT, ZERO_FEES, "Ion Vault Token", "IVT", INITIAL_DELAY, VAULT_ADMIN); + + vm.startPrank(vault.defaultAdmin()); + vault.grantRole(vault.OWNER_ROLE(), OWNER); + vault.grantRole(vault.ALLOCATOR_ROLE(), OWNER); + vm.stopPrank(); + + IIonPool[] memory markets = new IIonPool[](vault.MAX_SUPPORTED_MARKETS() + 1); + uint256[] memory allocationCaps = new uint256[](vault.MAX_SUPPORTED_MARKETS() + 1); + + for (uint8 i = 0; i < vault.MAX_SUPPORTED_MARKETS() + 1; i++) { + markets[i] = deployIonPool(BASE_ASSET, WEETH, address(this)); + allocationCaps[i] = 1 ether; + } + + vm.startPrank(OWNER); + vm.expectRevert(Vault.MaxSupportedMarketsReached.selector); + vault.addSupportedMarkets(markets, allocationCaps, markets, markets); + vm.stopPrank(); + } + function test_RemoveSingleSupportedMarket() public { uint256[] memory allocationCaps = new uint256[](1); allocationCaps[0] = 1e18; From 971e8f69bf3a192ea394c6fd55e5b31ab85e9998 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Sat, 11 May 2024 19:54:21 -0400 Subject: [PATCH 04/20] fix: if paused, the depositable and withdrawable amount must be zero --- src/vault/Vault.sol | 8 ++-- test/unit/concrete/vault/Vault.t.sol | 58 ++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/vault/Vault.sol b/src/vault/Vault.sol index fc3231db..04dd003e 100644 --- a/src/vault/Vault.sol +++ b/src/vault/Vault.sol @@ -16,7 +16,6 @@ import { Multicall } from "@openzeppelin/contracts/utils/Multicall.sol"; import { ReentrancyGuard } from "openzeppelin-contracts/contracts/utils/ReentrancyGuard.sol"; import { AccessControlDefaultAdminRules } from "@openzeppelin/contracts/access/extensions/AccessControlDefaultAdminRules.sol"; - /** * @title Ion Lending Vault * @author Molecular Labs @@ -28,6 +27,7 @@ import { AccessControlDefaultAdminRules } from * * @custom:security-contact security@molecularlabs.io */ + contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, ReentrancyGuard { using EnumerableSet for EnumerableSet.AddressSet; using Math for uint256; @@ -752,7 +752,6 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy newTotalSupply = totalSupply() + feeShares; assets = _convertToAssetsWithTotals(balanceOf(owner), newTotalSupply, newTotalAssets, Math.Rounding.Floor); - assets -= _simulateWithdrawIon(assets); } @@ -897,9 +896,10 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy * @return The max amount of assets withdrawable from this IonPool. */ function _withdrawable(IIonPool pool) internal view returns (uint256) { + if (pool.paused()) return 0; + uint256 currentSupplied = pool.getUnderlyingClaimOf(address(this)); uint256 availableLiquidity = uint256(pool.extsload(ION_POOL_LIQUIDITY_SLOT)); - return Math.min(currentSupplied, availableLiquidity); } @@ -910,6 +910,8 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy * @return The max amount of assets depositable to this IonPool. */ function _depositable(IIonPool pool) internal view returns (uint256) { + if (pool.paused()) return 0; + uint256 allocationCapDiff = _zeroFloorSub(caps[pool], pool.getUnderlyingClaimOf(address(this))); uint256 supplyCapDiff = _zeroFloorSub(uint256(pool.extsload(ION_POOL_SUPPLY_CAP_SLOT)), pool.getTotalUnderlyingClaims()); diff --git a/test/unit/concrete/vault/Vault.t.sol b/test/unit/concrete/vault/Vault.t.sol index cafcdb22..7a460dc8 100644 --- a/test/unit/concrete/vault/Vault.t.sol +++ b/test/unit/concrete/vault/Vault.t.sol @@ -1496,6 +1496,64 @@ contract VaultERC4626ExternalViews is VaultSharedSetup { function test_MaxRedeem() public { } + function test_MaxWithdrawWithPausedPools() public { + uint256[] memory allocationCaps = new uint256[](3); + allocationCaps[0] = 10e18; + allocationCaps[1] = 20e18; + allocationCaps[2] = 30e18; + + vm.prank(OWNER); + vault.updateAllocationCaps(markets, allocationCaps); + + uint256 depositAmt = 35e18; + setERC20Balance(address(BASE_ASSET), address(this), depositAmt); + vault.deposit(depositAmt, address(this)); + + uint256 maxWithdrawBeforePause = vault.maxWithdraw(address(this)); + + weEthIonPool.pause(); + uint256 maxWithdrawAfterPause = vault.maxWithdraw(address(this)); + + rsEthIonPool.pause(); + uint256 maxWithdrawAfterSecondPause = vault.maxWithdraw(address(this)); + + rswEthIonPool.pause(); + uint256 maxWithdrawAfterThirdPause = vault.maxWithdraw(address(this)); + + assertEq(maxWithdrawBeforePause, depositAmt, "max withdraw before pause"); + assertEq(maxWithdrawAfterPause, depositAmt - 10e18, "max withdraw after pause"); + assertEq(maxWithdrawAfterSecondPause, depositAmt - 30e18, "max withdraw after second pause"); + assertEq(maxWithdrawAfterThirdPause, 0, "max withdraw after third pause"); + } + + function test_MaxDepositWithPausedPools() public { + uint256[] memory allocationCaps = new uint256[](3); + allocationCaps[0] = 10e18; + allocationCaps[1] = 20e18; + allocationCaps[2] = 30e18; + + vm.prank(OWNER); + vault.updateAllocationCaps(markets, allocationCaps); + + uint256 maxDepositBeforePause = vault.maxDeposit(NULL); + + weEthIonPool.pause(); + uint256 maxDepositAfterPause = vault.maxDeposit(NULL); + + rsEthIonPool.pause(); + uint256 maxDepositAfterSecondPause = vault.maxDeposit(NULL); + + rswEthIonPool.pause(); + uint256 maxDepositAfterThirdPause = vault.maxDeposit(NULL); + + assertEq(maxDepositBeforePause, 60e18, "max deposit before pause"); + assertEq(maxDepositAfterPause, 50e18, "max deposit after pause"); + assertEq(maxDepositAfterSecondPause, 30e18, "max deposit after second pause"); + assertEq(maxDepositAfterThirdPause, 0, "max deposit after third pause"); + } + + function test_WithdrawWithPausedPools() public { } + // --- Previews --- // Check the difference between preview and actual From d9b87b2abe58952e00bca956fbc01f26f731b563 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Sat, 11 May 2024 23:10:30 -0400 Subject: [PATCH 05/20] fix: divide by zero in interest rate module when totalEthSupply is too small now returns minimum base rate instead of reverting --- src/InterestRate.sol | 11 ++++- test/unit/concrete/IonPool.t.sol | 76 ++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/InterestRate.sol b/src/InterestRate.sol index b6a5b25f..d4c7c841 100644 --- a/src/InterestRate.sol +++ b/src/InterestRate.sol @@ -325,9 +325,16 @@ contract InterestRate { if (distributionFactor == 0) { return (ilkData.minimumKinkRate, ilkData.reserveFactor.scaleUpToRay(4)); } + + uint256 totalEthSupplyScaled = totalEthSupply.wadMulDown(distributionFactor.scaleUpToWad(4)); + // If the `totalEthSupply` is small enough to truncate to zero, then + // simply return the minimum base rate. + if (totalEthSupplyScaled == 0) { + return (ilkData.minimumBaseRate, ilkData.reserveFactor.scaleUpToRay(4)); + } + // [RAD] / [WAD] = [RAY] - uint256 utilizationRate = - totalEthSupply == 0 ? 0 : totalIlkDebt / (totalEthSupply.wadMulDown(distributionFactor.scaleUpToWad(4))); + uint256 utilizationRate = totalEthSupply == 0 ? 0 : totalIlkDebt / totalEthSupplyScaled; // Avoid stack too deep uint256 adjustedBelowKinkSlope; diff --git a/test/unit/concrete/IonPool.t.sol b/test/unit/concrete/IonPool.t.sol index 0863f53a..87b5eb46 100644 --- a/test/unit/concrete/IonPool.t.sol +++ b/test/unit/concrete/IonPool.t.sol @@ -1192,6 +1192,82 @@ contract IonPool_InterestTest is IonPoolSharedSetup, IIonPoolEvents { previousRates[i] = rate; } } + + // If distribution factor is zero, should return + // minimum kink rate. + function test_DivideByZeroWhenDistributionFactorIsZero() public { + IlkData[] memory ilkConfigs = new IlkData[](2); + uint16[] memory distributionFactors = new uint16[](2); + distributionFactors[0] = 0; + distributionFactors[1] = 1e4; + + uint96 minimumKinkRate = 4_062_570_058_138_700_000; + for (uint8 i; i != 2; ++i) { + IlkData memory ilkConfig = IlkData({ + adjustedProfitMargin: 0, + minimumKinkRate: minimumKinkRate, + reserveFactor: 0, + adjustedBaseRate: 0, + minimumBaseRate: 0, + optimalUtilizationRate: 9000, + distributionFactor: distributionFactors[i], + adjustedAboveKinkSlope: 0, + minimumAboveKinkSlope: 0 + }); + ilkConfigs[i] = ilkConfig; + } + + interestRateModule = new InterestRate(ilkConfigs, apyOracle); + + vm.warp(block.timestamp + 1 days); + + (uint256 zeroDistFactorBorrowRate,) = interestRateModule.calculateInterestRate(0, 10e45, 100e18); // 10% + // utilization + assertEq(zeroDistFactorBorrowRate, minimumKinkRate, "borrow rate should be minimum kink rate"); + + (uint256 nonZeroDistFactorBorrowRate,) = interestRateModule.calculateInterestRate(1, 100e45, 100e18); // 90% + // utilization + assertApproxEqAbs( + nonZeroDistFactorBorrowRate, minimumKinkRate, 1, "borrow rate at any util should be minimum kink rate" + ); + } + + // If scaling total eth supply with distribution factor truncates to zero, + // should return minimum base rate. + function test_DivideByZeroWhenTotalEthSupplyIsSmall() public { + IlkData[] memory ilkConfigs = new IlkData[](2); + uint16[] memory distributionFactors = new uint16[](2); + distributionFactors[0] = 0.5e4; + distributionFactors[1] = 0.5e4; + + uint96 minimumKinkRate = 4_062_570_058_138_700_000; + uint96 minimumBaseRate = 1_580_630_071_273_960_000; + for (uint8 i; i != 2; ++i) { + IlkData memory ilkConfig = IlkData({ + adjustedProfitMargin: 0, + minimumKinkRate: minimumKinkRate, + reserveFactor: 0, + adjustedBaseRate: 0, + minimumBaseRate: minimumBaseRate, + optimalUtilizationRate: 9000, + distributionFactor: distributionFactors[i], + adjustedAboveKinkSlope: 0, + minimumAboveKinkSlope: 0 + }); + ilkConfigs[i] = ilkConfig; + } + + interestRateModule = new InterestRate(ilkConfigs, apyOracle); + + vm.warp(block.timestamp + 1 days); + + (uint256 borrowRate,) = interestRateModule.calculateInterestRate(0, 0, 1); // dust amount of eth supply + assertEq(borrowRate, minimumBaseRate, "borrow rate should be minimum base rate"); + + (uint256 borrowRateWithoutTruncation,) = interestRateModule.calculateInterestRate(1, 90e45, 100e18); // 90% + // utilization + assertApproxEqAbs(borrowRateWithoutTruncation, minimumKinkRate, 1, "borrow rate without truncation"); + } } contract IonPool_AdminTest is IonPoolSharedSetup { From aade327cce25ac4c84c712bfc963a165ecd7a9c4 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Sun, 12 May 2024 01:45:08 -0400 Subject: [PATCH 06/20] fix: revert the non-rebasing model of RewardToken to rebasing; changes must only be renaming function names --- src/interfaces/IIonPool.sol | 1 - src/token/RewardToken.sol | 8 --- src/vault/Vault.sol | 16 +---- test/unit/concrete/vault/Vault.t.sol | 103 --------------------------- 4 files changed, 1 insertion(+), 127 deletions(-) diff --git a/src/interfaces/IIonPool.sol b/src/interfaces/IIonPool.sol index b7b9e587..b28d071f 100644 --- a/src/interfaces/IIonPool.sol +++ b/src/interfaces/IIonPool.sol @@ -234,5 +234,4 @@ interface IIonPool { function getTotalUnderlyingClaims() external view returns (uint256); function getUnderlyingClaimOf(address user) external view returns (uint256); function extsload(bytes32 slot) external view returns (bytes32); - function balanceOfUnaccrued(address user) external view returns (uint256); } diff --git a/src/token/RewardToken.sol b/src/token/RewardToken.sol index c1887128..aa9c3120 100644 --- a/src/token/RewardToken.sol +++ b/src/token/RewardToken.sol @@ -459,14 +459,6 @@ abstract contract RewardToken is return $._normalizedBalances[user].rayMulDown($.supplyFactor + totalSupplyFactorIncrease); } - /** - * @dev Current claim of the underlying token without accounting for interest to be accrued. - */ - function balanceOfUnaccrued(address user) public view returns (uint256) { - RewardTokenStorage storage $ = _getRewardTokenStorage(); - return $._normalizedBalances[user].rayMulDown($.supplyFactor); - } - /** * @dev Accounting is done in normalized balances * @param user to get normalized balance of diff --git a/src/vault/Vault.sol b/src/vault/Vault.sol index a3569fcd..6b42ebc7 100644 --- a/src/vault/Vault.sol +++ b/src/vault/Vault.sol @@ -115,7 +115,6 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy */ function updateFeePercentage(uint256 _feePercentage) external onlyRole(OWNER_ROLE) { if (_feePercentage > RAY) revert InvalidFeePercentage(); - _accrueFee(); feePercentage = _feePercentage; } @@ -344,8 +343,6 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy // to the user from the previous function scope. if (pool != IDLE) { pool.withdraw(address(this), transferAmt); - } else { - currentIdleDeposits -= transferAmt; } totalWithdrawn += transferAmt; @@ -375,8 +372,6 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy // contract. if (pool != IDLE) { pool.supply(address(this), transferAmt, new bytes32[](0)); - } else { - currentIdleDeposits += transferAmt; } totalSupplied += transferAmt; @@ -660,16 +655,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy for (uint256 i; i != _supportedMarketsLength;) { IIonPool pool = IIonPool(supportedMarkets.at(i)); - uint256 assetsInPool; - if (pool == IDLE) { - assetsInPool = BASE_ASSET.balanceOf(address(this)); - } else { - if (pool.paused()) { - assetsInPool = pool.balanceOfUnaccrued(address(this)); - } else { - assetsInPool = pool.balanceOf(address(this)); - } - } + uint256 assetsInPool = pool == IDLE ? BASE_ASSET.balanceOf(address(this)) : pool.balanceOf(address(this)); assets += assetsInPool; diff --git a/test/unit/concrete/vault/Vault.t.sol b/test/unit/concrete/vault/Vault.t.sol index 60c3168a..31ad2d14 100644 --- a/test/unit/concrete/vault/Vault.t.sol +++ b/test/unit/concrete/vault/Vault.t.sol @@ -1425,109 +1425,6 @@ contract VaultERC4626ExternalViews is VaultSharedSetup { super.setUp(); } - function test_TotalAssetsWithSinglePausedIonPool() public { - weEthIonPool.updateSupplyCap(type(uint256).max); - weEthIonPool.updateIlkDebtCeiling(0, type(uint256).max); - - supply(address(this), weEthIonPool, 1000e18); - borrow(address(this), weEthIonPool, weEthGemJoin, 100e18, 70e18); - - uint256[] memory allocationCaps = new uint256[](3); - allocationCaps[0] = 20e18; - allocationCaps[1] = 0; - allocationCaps[2] = 0; - - vm.prank(OWNER); - vault.updateAllocationCaps(markets, allocationCaps); - - uint256 depositAmt = 10e18; - setERC20Balance(address(BASE_ASSET), address(this), depositAmt); - vault.deposit(depositAmt, address(this)); - - assertEq(weEthIonPool.balanceOf(address(vault)), depositAmt, "weEthIonPool balance"); - // pause the weEthIonPool - weEthIonPool.pause(); - - vm.warp(block.timestamp + 365 days); - - assertGt(weEthIonPool.balanceOf(address(vault)), depositAmt, "weEthIonPool accrues interest"); - assertLt( - weEthIonPool.balanceOfUnaccrued(address(vault)), - weEthIonPool.balanceOf(address(vault)), - "weEthIonPool unaccrued balance" - ); - assertTrue(weEthIonPool.paused(), "weEthIonPool is paused"); - - uint256 totalAssets = vault.totalAssets(); - assertEq(totalAssets, depositAmt, "total assets with paused IonPool does not include interest"); - } - - function test_TotalAssetsWithMultiplePausedIonPools() public { - // Make sure every pool has debt to accrue interest from - uint256 initialSupplyAmt = 1000e18; - weEthIonPool.updateSupplyCap(type(uint256).max); - rsEthIonPool.updateSupplyCap(type(uint256).max); - rswEthIonPool.updateSupplyCap(type(uint256).max); - - weEthIonPool.updateIlkDebtCeiling(0, type(uint256).max); - rsEthIonPool.updateIlkDebtCeiling(0, type(uint256).max); - rswEthIonPool.updateIlkDebtCeiling(0, type(uint256).max); - - supply(address(this), weEthIonPool, initialSupplyAmt); - borrow(address(this), weEthIonPool, weEthGemJoin, 100e18, 70e18); - - supply(address(this), rsEthIonPool, initialSupplyAmt); - borrow(address(this), rsEthIonPool, rsEthGemJoin, 100e18, 70e18); - - supply(address(this), rswEthIonPool, initialSupplyAmt); - borrow(address(this), rswEthIonPool, rswEthGemJoin, 100e18, 70e18); - - uint256[] memory allocationCaps = new uint256[](3); - uint256 weEthIonPoolAmt = 10e18; - uint256 rsEthIonPoolAmt = 20e18; - uint256 rswEthIonPoolAmt = 30e18; - allocationCaps[0] = weEthIonPoolAmt; - allocationCaps[1] = rsEthIonPoolAmt; - allocationCaps[2] = rswEthIonPoolAmt; - - vm.prank(OWNER); - vault.updateAllocationCaps(markets, allocationCaps); - - uint256 depositAmt = 60e18; - setERC20Balance(address(BASE_ASSET), address(this), depositAmt); - vault.deposit(depositAmt, address(this)); - - assertEq(weEthIonPool.balanceOf(address(vault)), weEthIonPoolAmt, "weEthIonPool balance"); - assertEq(rsEthIonPool.balanceOf(address(vault)), rsEthIonPoolAmt, "rsEthIonPool balance"); - assertEq(rswEthIonPool.balanceOf(address(vault)), rswEthIonPoolAmt, "rswEthIonPool balance"); - - weEthIonPool.pause(); - // NOTE rsEthIonPool is not paused - rswEthIonPool.pause(); - - assertTrue(weEthIonPool.paused(), "weEthIonPool is paused"); - assertFalse(rsEthIonPool.paused(), "rsEthIonPool is not paused"); - assertTrue(rswEthIonPool.paused(), "rswEthIonPool is paused"); - - vm.warp(block.timestamp + 365 days); - - assertGt(weEthIonPool.balanceOf(address(vault)), weEthIonPoolAmt, "weEthIonPool balance increases"); - assertGt(rsEthIonPool.balanceOf(address(vault)), rsEthIonPoolAmt, "rsEthIonPool balance does not change"); - assertGt(rswEthIonPool.balanceOf(address(vault)), rswEthIonPoolAmt, "rswEthIonPool balance increases"); - - // The 'unaccrued' values should not change - assertEq(weEthIonPool.balanceOfUnaccrued(address(vault)), weEthIonPoolAmt, "weEthIonPool balance"); - assertEq(rsEthIonPool.balanceOfUnaccrued(address(vault)), rsEthIonPoolAmt, "rsEthIonPool balance"); - assertEq(rswEthIonPool.balanceOfUnaccrued(address(vault)), rswEthIonPoolAmt, "rswEthIonPool balance"); - - uint256 expectedTotalAssets = weEthIonPool.balanceOfUnaccrued(address(vault)) - + rsEthIonPool.balanceOf(address(vault)) + rswEthIonPool.balanceOfUnaccrued(address(vault)); - - assertEq( - vault.totalAssets(), expectedTotalAssets, "total assets without accounting for interest in paused IonPools" - ); - } - // --- Max --- // Get max and submit max transactions From c39ad6c7dcee66fe7d0eccdbe2de84a2c9427ca7 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Sun, 12 May 2024 02:24:46 -0400 Subject: [PATCH 07/20] fix: when calculating total assets, query unaccrued balanceOf if the pool is paused --- src/interfaces/IIonPool.sol | 1 + src/token/RewardToken.sol | 8 +++ src/vault/Vault.sol | 11 ++- test/unit/concrete/vault/Vault.t.sol | 103 +++++++++++++++++++++++++++ 4 files changed, 122 insertions(+), 1 deletion(-) diff --git a/src/interfaces/IIonPool.sol b/src/interfaces/IIonPool.sol index b28d071f..b7b9e587 100644 --- a/src/interfaces/IIonPool.sol +++ b/src/interfaces/IIonPool.sol @@ -234,4 +234,5 @@ interface IIonPool { function getTotalUnderlyingClaims() external view returns (uint256); function getUnderlyingClaimOf(address user) external view returns (uint256); function extsload(bytes32 slot) external view returns (bytes32); + function balanceOfUnaccrued(address user) external view returns (uint256); } diff --git a/src/token/RewardToken.sol b/src/token/RewardToken.sol index aa9c3120..c1887128 100644 --- a/src/token/RewardToken.sol +++ b/src/token/RewardToken.sol @@ -459,6 +459,14 @@ abstract contract RewardToken is return $._normalizedBalances[user].rayMulDown($.supplyFactor + totalSupplyFactorIncrease); } + /** + * @dev Current claim of the underlying token without accounting for interest to be accrued. + */ + function balanceOfUnaccrued(address user) public view returns (uint256) { + RewardTokenStorage storage $ = _getRewardTokenStorage(); + return $._normalizedBalances[user].rayMulDown($.supplyFactor); + } + /** * @dev Accounting is done in normalized balances * @param user to get normalized balance of diff --git a/src/vault/Vault.sol b/src/vault/Vault.sol index 6b42ebc7..0b699cfa 100644 --- a/src/vault/Vault.sol +++ b/src/vault/Vault.sol @@ -655,7 +655,16 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy for (uint256 i; i != _supportedMarketsLength;) { IIonPool pool = IIonPool(supportedMarkets.at(i)); - uint256 assetsInPool = pool == IDLE ? BASE_ASSET.balanceOf(address(this)) : pool.balanceOf(address(this)); + uint256 assetsInPool; + if (pool == IDLE) { + assetsInPool = BASE_ASSET.balanceOf(address(this)); + } else { + if (pool.paused()) { + assetsInPool = pool.balanceOfUnaccrued(address(this)); + } else { + assetsInPool = pool.balanceOf(address(this)); + } + } assets += assetsInPool; diff --git a/test/unit/concrete/vault/Vault.t.sol b/test/unit/concrete/vault/Vault.t.sol index 31ad2d14..60c3168a 100644 --- a/test/unit/concrete/vault/Vault.t.sol +++ b/test/unit/concrete/vault/Vault.t.sol @@ -1425,6 +1425,109 @@ contract VaultERC4626ExternalViews is VaultSharedSetup { super.setUp(); } + function test_TotalAssetsWithSinglePausedIonPool() public { + weEthIonPool.updateSupplyCap(type(uint256).max); + weEthIonPool.updateIlkDebtCeiling(0, type(uint256).max); + + supply(address(this), weEthIonPool, 1000e18); + borrow(address(this), weEthIonPool, weEthGemJoin, 100e18, 70e18); + + uint256[] memory allocationCaps = new uint256[](3); + allocationCaps[0] = 20e18; + allocationCaps[1] = 0; + allocationCaps[2] = 0; + + vm.prank(OWNER); + vault.updateAllocationCaps(markets, allocationCaps); + + uint256 depositAmt = 10e18; + setERC20Balance(address(BASE_ASSET), address(this), depositAmt); + vault.deposit(depositAmt, address(this)); + + assertEq(weEthIonPool.balanceOf(address(vault)), depositAmt, "weEthIonPool balance"); + // pause the weEthIonPool + weEthIonPool.pause(); + + vm.warp(block.timestamp + 365 days); + + assertGt(weEthIonPool.balanceOf(address(vault)), depositAmt, "weEthIonPool accrues interest"); + assertLt( + weEthIonPool.balanceOfUnaccrued(address(vault)), + weEthIonPool.balanceOf(address(vault)), + "weEthIonPool unaccrued balance" + ); + assertTrue(weEthIonPool.paused(), "weEthIonPool is paused"); + + uint256 totalAssets = vault.totalAssets(); + assertEq(totalAssets, depositAmt, "total assets with paused IonPool does not include interest"); + } + + function test_TotalAssetsWithMultiplePausedIonPools() public { + // Make sure every pool has debt to accrue interest from + uint256 initialSupplyAmt = 1000e18; + weEthIonPool.updateSupplyCap(type(uint256).max); + rsEthIonPool.updateSupplyCap(type(uint256).max); + rswEthIonPool.updateSupplyCap(type(uint256).max); + + weEthIonPool.updateIlkDebtCeiling(0, type(uint256).max); + rsEthIonPool.updateIlkDebtCeiling(0, type(uint256).max); + rswEthIonPool.updateIlkDebtCeiling(0, type(uint256).max); + + supply(address(this), weEthIonPool, initialSupplyAmt); + borrow(address(this), weEthIonPool, weEthGemJoin, 100e18, 70e18); + + supply(address(this), rsEthIonPool, initialSupplyAmt); + borrow(address(this), rsEthIonPool, rsEthGemJoin, 100e18, 70e18); + + supply(address(this), rswEthIonPool, initialSupplyAmt); + borrow(address(this), rswEthIonPool, rswEthGemJoin, 100e18, 70e18); + + uint256[] memory allocationCaps = new uint256[](3); + uint256 weEthIonPoolAmt = 10e18; + uint256 rsEthIonPoolAmt = 20e18; + uint256 rswEthIonPoolAmt = 30e18; + allocationCaps[0] = weEthIonPoolAmt; + allocationCaps[1] = rsEthIonPoolAmt; + allocationCaps[2] = rswEthIonPoolAmt; + + vm.prank(OWNER); + vault.updateAllocationCaps(markets, allocationCaps); + + uint256 depositAmt = 60e18; + setERC20Balance(address(BASE_ASSET), address(this), depositAmt); + vault.deposit(depositAmt, address(this)); + + assertEq(weEthIonPool.balanceOf(address(vault)), weEthIonPoolAmt, "weEthIonPool balance"); + assertEq(rsEthIonPool.balanceOf(address(vault)), rsEthIonPoolAmt, "rsEthIonPool balance"); + assertEq(rswEthIonPool.balanceOf(address(vault)), rswEthIonPoolAmt, "rswEthIonPool balance"); + + weEthIonPool.pause(); + // NOTE rsEthIonPool is not paused + rswEthIonPool.pause(); + + assertTrue(weEthIonPool.paused(), "weEthIonPool is paused"); + assertFalse(rsEthIonPool.paused(), "rsEthIonPool is not paused"); + assertTrue(rswEthIonPool.paused(), "rswEthIonPool is paused"); + + vm.warp(block.timestamp + 365 days); + + assertGt(weEthIonPool.balanceOf(address(vault)), weEthIonPoolAmt, "weEthIonPool balance increases"); + assertGt(rsEthIonPool.balanceOf(address(vault)), rsEthIonPoolAmt, "rsEthIonPool balance does not change"); + assertGt(rswEthIonPool.balanceOf(address(vault)), rswEthIonPoolAmt, "rswEthIonPool balance increases"); + + // The 'unaccrued' values should not change + assertEq(weEthIonPool.balanceOfUnaccrued(address(vault)), weEthIonPoolAmt, "weEthIonPool balance"); + assertEq(rsEthIonPool.balanceOfUnaccrued(address(vault)), rsEthIonPoolAmt, "rsEthIonPool balance"); + assertEq(rswEthIonPool.balanceOfUnaccrued(address(vault)), rswEthIonPoolAmt, "rswEthIonPool balance"); + + uint256 expectedTotalAssets = weEthIonPool.balanceOfUnaccrued(address(vault)) + + rsEthIonPool.balanceOf(address(vault)) + rswEthIonPool.balanceOfUnaccrued(address(vault)); + + assertEq( + vault.totalAssets(), expectedTotalAssets, "total assets without accounting for interest in paused IonPools" + ); + } + // --- Max --- // Get max and submit max transactions From 1c6eefdf7158adf777d73b1bdfe8b5a608e4c0c5 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Sun, 12 May 2024 02:30:22 -0400 Subject: [PATCH 08/20] fix: update currentIdleDeposits counter when reallocating to and from IDLE pool --- src/vault/Vault.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/vault/Vault.sol b/src/vault/Vault.sol index 6b42ebc7..81dc10db 100644 --- a/src/vault/Vault.sol +++ b/src/vault/Vault.sol @@ -343,6 +343,8 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy // to the user from the previous function scope. if (pool != IDLE) { pool.withdraw(address(this), transferAmt); + } else { + currentIdleDeposits -= transferAmt; } totalWithdrawn += transferAmt; @@ -372,6 +374,8 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy // contract. if (pool != IDLE) { pool.supply(address(this), transferAmt, new bytes32[](0)); + } else { + currentIdleDeposits += transferAmt; } totalSupplied += transferAmt; From 51102cc09771d6257485945283bae164aa1656e2 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Sun, 12 May 2024 02:37:38 -0400 Subject: [PATCH 09/20] fix: accrue fee before changing the feePercentage --- src/vault/Vault.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vault/Vault.sol b/src/vault/Vault.sol index 6b42ebc7..514019b9 100644 --- a/src/vault/Vault.sol +++ b/src/vault/Vault.sol @@ -115,6 +115,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy */ function updateFeePercentage(uint256 _feePercentage) external onlyRole(OWNER_ROLE) { if (_feePercentage > RAY) revert InvalidFeePercentage(); + _accrueFee(); feePercentage = _feePercentage; } From 24f904ceaaff0fae6a6fea9961374366d3e60831 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Sun, 12 May 2024 03:02:08 -0400 Subject: [PATCH 10/20] fix: the total supply query now accounts for the treasury mint amount --- src/token/RewardToken.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/token/RewardToken.sol b/src/token/RewardToken.sol index aa9c3120..ff21fbd8 100644 --- a/src/token/RewardToken.sol +++ b/src/token/RewardToken.sol @@ -519,9 +519,9 @@ abstract contract RewardToken is return 0; } - (uint256 totalSupplyFactorIncrease,,,,) = calculateRewardAndDebtDistribution(); + (uint256 totalSupplyFactorIncrease, uint256 totalTreasuryMintAmount,,,) = calculateRewardAndDebtDistribution(); - return _normalizedTotalSupply.rayMulDown($.supplyFactor + totalSupplyFactorIncrease); + return _normalizedTotalSupply.rayMulDown($.supplyFactor + totalSupplyFactorIncrease) + totalTreasuryMintAmount; } function normalizedTotalSupplyUnaccrued() public view returns (uint256) { From 9a8acc2b2f808f68705b7c91dcf2b3e6c428b2bf Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Sun, 12 May 2024 03:42:36 -0400 Subject: [PATCH 11/20] fix: the _maxWithdraw includes fee shares --- src/vault/Vault.sol | 7 ++++++- test/unit/fuzz/vault/Vault.t.sol | 5 +++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/vault/Vault.sol b/src/vault/Vault.sol index 6b42ebc7..d79693cb 100644 --- a/src/vault/Vault.sol +++ b/src/vault/Vault.sol @@ -750,7 +750,12 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy (feeShares, newTotalAssets) = _accruedFeeShares(); newTotalSupply = totalSupply() + feeShares; - assets = _convertToAssetsWithTotals(balanceOf(owner), newTotalSupply, newTotalAssets, Math.Rounding.Floor); + uint256 shareBalances = balanceOf(owner); + if (owner == feeRecipient) { + shareBalances += feeShares; + } + + assets = _convertToAssetsWithTotals(shareBalances, newTotalSupply, newTotalAssets, Math.Rounding.Floor); assets -= _simulateWithdrawIon(assets); } diff --git a/test/unit/fuzz/vault/Vault.t.sol b/test/unit/fuzz/vault/Vault.t.sol index 32cc524f..ef5c6767 100644 --- a/test/unit/fuzz/vault/Vault.t.sol +++ b/test/unit/fuzz/vault/Vault.t.sol @@ -124,8 +124,9 @@ contract VaultWithYieldAndFee_Fuzz is VaultSharedSetup { // expected resulting state uint256 expectedFeeAssets = interestAccrued.mulDiv(feePerc, RAY); - uint256 expectedFeeShares = - expectedFeeAssets.mulDiv(vault.totalSupply(), newTotalAssets - expectedFeeAssets, Math.Rounding.Floor); + uint256 expectedFeeShares = expectedFeeAssets.mulDiv( + vault.totalSupply() + 1, newTotalAssets - expectedFeeAssets + 1, Math.Rounding.Floor + ); uint256 expectedUserAssets = prevUserAssets + interestAccrued.mulDiv(RAY - feePerc, RAY); From 3fc0bed2458077c2af6450242a0ae2b5e4473175 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Mon, 13 May 2024 04:38:10 -0400 Subject: [PATCH 12/20] fix: if the supply or withdraw reverts, skip the market and move onto the next iteration --- src/vault/Vault.sol | 21 +++++--- test/unit/concrete/vault/Vault.t.sol | 77 +++++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/src/vault/Vault.sol b/src/vault/Vault.sol index 6b42ebc7..8a925443 100644 --- a/src/vault/Vault.sol +++ b/src/vault/Vault.sol @@ -421,12 +421,16 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy // For the IDLE pool, decrement the accumulator at the end of this // loop, but no external interactions need to be made as the assets - // are already on this contract' balance. + // are already on this contract' balance. If the pool supply + // reverts, simply skip to the next iteration. if (pool != IDLE) { - pool.supply(address(this), toSupply, new bytes32[](0)); + try pool.supply(address(this), toSupply, new bytes32[](0)) { + assets -= toSupply; + } catch { } + } else { + assets -= toSupply; } - assets -= toSupply; if (assets == 0) return; } @@ -458,12 +462,17 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy uint256 toWithdraw = Math.min(withdrawable, assets); // For the `IDLE` pool, they are already on this contract's - // balance. Update `assets` accumulator but don't actually transfer. + // balance. Update `assets` accumulator but don't actually + // transfer. If the pool withdraw reverts, simply skip to the + // next iteration. if (pool != IDLE) { - pool.withdraw(address(this), toWithdraw); + try pool.withdraw(address(this), toWithdraw) { + assets -= toWithdraw; + } catch { } + } else { + assets -= toWithdraw; } - assets -= toWithdraw; if (assets == 0) return; } diff --git a/test/unit/concrete/vault/Vault.t.sol b/test/unit/concrete/vault/Vault.t.sol index 31ad2d14..26e4e763 100644 --- a/test/unit/concrete/vault/Vault.t.sol +++ b/test/unit/concrete/vault/Vault.t.sol @@ -793,6 +793,46 @@ abstract contract VaultDeposit is VaultSharedSetup { assertEq(assetsDeposited, expectedDepositAmount, "mint return value"); } + function test_Deposit_SkipPauseReverts() public { + // If the market is paused, then the deposit iteration should skip it. + updateAllocationCaps(vault, 10 ether, 20 ether, 30 ether); + + uint256 depositAmt = 30 ether; + setERC20Balance(address(BASE_ASSET), address(this), depositAmt); + + // The second market is paused. + rsEthIonPool.pause(); + + // 10 ether in weEthIonPool, 20 ether in rswEthIonPool + vault.deposit(depositAmt, address(this)); + + assertEq( + weEthIonPool.balanceOf(address(vault)), + claimAfterDeposit(0, 10 ether, weEthIonPool.supplyFactor()), + "weEthIonPool balance" + ); + assertEq(rsEthIonPool.balanceOf(address(vault)), 0, "rsEthIonPool balance"); + assertEq( + rswEthIonPool.balanceOf(address(vault)), + claimAfterDeposit(0, 20 ether, rswEthIonPool.supplyFactor()), + "rswEthIonPool balance" + ); + } + + function test_Deposit_AllMarketsPaused() public { + updateAllocationCaps(vault, 10 ether, 20 ether, 30 ether); + + weEthIonPool.pause(); + rsEthIonPool.pause(); + rswEthIonPool.pause(); + + uint256 depositAmt = 30 ether; + setERC20Balance(address(BASE_ASSET), address(this), depositAmt); + + vm.expectRevert(Vault.AllSupplyCapsReached.selector); + vault.deposit(depositAmt, address(this)); + } + function test_Mint_AllMarkets() public { } } @@ -930,6 +970,42 @@ abstract contract VaultWithdraw is VaultSharedSetup { vault.withdraw(withdrawAmount, address(this), address(this)); } + function test_Withdraw_SkipPauseReverts() public { + uint256 depositAmt = 6e18; + uint256 withdrawAmt = 3e18; + + updateAllocationCaps(vault, 1e18, 2e18, 3e18); + + // Deposit 1e18 to weETH, 2e18 to rsETH, 3e18 to rswETH + // rsETH is paused + // Withdraw 1e18 from weETH, 0 from rsETH, 2e18 from rswETH + + setERC20Balance(address(BASE_ASSET), address(this), depositAmt); + vault.deposit(depositAmt, address(this)); + + uint256 initialWeEthIonPoolDeposit = weEthIonPool.balanceOf(address(vault)); + uint256 initialRsEthIonPoolDeposit = rsEthIonPool.balanceOf(address(vault)); + uint256 initialRswEthIonPoolDeposit = rswEthIonPool.balanceOf(address(vault)); + + rsEthIonPool.pause(); + + vault.withdraw(withdrawAmt, address(this), address(this)); + + uint256 rswEthIonPoolWithdrawAmt = withdrawAmt - initialWeEthIonPoolDeposit; + uint256 expectedRswEthIonPoolDeposit = initialRswEthIonPoolDeposit - rswEthIonPoolWithdrawAmt; + uint256 rswEthIonPoolDeposit = rswEthIonPool.balanceOf(address(vault)); + + assertEq(weEthIonPool.balanceOf(address(vault)), 0, "weEthIonPool balance"); + assertEq( + rsEthIonPool.balanceOf(address(vault)), initialRsEthIonPoolDeposit, "rsEthIonPool deposit should not change" + ); + assertLe( + expectedRswEthIonPoolDeposit - rswEthIonPoolDeposit, + rswEthIonPool.supplyFactor() / RAY, + "rswEthIonPool balance" + ); + } + // try to deposit and withdraw same amounts function test_Withdraw_FullWithdraw() public { } @@ -1462,7 +1538,6 @@ contract VaultERC4626ExternalViews is VaultSharedSetup { vault.updateAllocationCaps(markets, allocationCaps); uint256 maxMintShares = vault.maxMint(NULL); - console2.log("maxMintShares: ", maxMintShares); setERC20Balance(address(BASE_ASSET), address(this), 60e18); vault.mint(maxMintShares, address(this)); From 0a29e38d929be188895da6a3305197ce4591938c Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Sun, 12 May 2024 01:19:37 -0400 Subject: [PATCH 13/20] test: show that depositing the allocation cap diff or supply cap diff cannot violate either max caps --- test/helpers/VaultSharedSetup.sol | 6 +++ test/unit/fuzz/vault/Vault.t.sol | 73 +++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/test/helpers/VaultSharedSetup.sol b/test/helpers/VaultSharedSetup.sol index 39618c79..bbf85f5f 100644 --- a/test/helpers/VaultSharedSetup.sol +++ b/test/helpers/VaultSharedSetup.sol @@ -284,4 +284,10 @@ contract VaultSharedSetup is IonPoolSharedSetup { IonPoolExposed(address(rsEthIonPool)).setSupplyFactor(7.1336673e27); IonPoolExposed(address(rswEthIonPool)).setSupplyFactor(10.1336673e27); } + + function _zeroFloorSub(uint256 x, uint256 y) internal pure returns (uint256 z) { + assembly { + z := mul(gt(x, y), sub(x, y)) + } + } } diff --git a/test/unit/fuzz/vault/Vault.t.sol b/test/unit/fuzz/vault/Vault.t.sol index 32cc524f..5e8d4a7f 100644 --- a/test/unit/fuzz/vault/Vault.t.sol +++ b/test/unit/fuzz/vault/Vault.t.sol @@ -10,6 +10,7 @@ import { console2 } from "forge-std/console2.sol"; import { IERC20 } from "openzeppelin-contracts/contracts/interfaces/IERC20.sol"; using Math for uint256; +using WadRayMath for uint256; contract Vault_Fuzz is VaultSharedSetup { function setUp() public override { @@ -55,6 +56,78 @@ contract Vault_Fuzz is VaultSharedSetup { assertEq(normalizedAmt, sharesToBurn); } + + // NOTE Supplying the diff can revert if the normalized mint amount + // truncates to zero. Otherwise, it should be impossible to supply the + // 'diff' and end up violating the supply cap. + + function testFuzz_DepositToFillSupplyCap(uint256 assets, uint256 supplyFactor) public { + supplyFactor = bound(supplyFactor, 1e27, 10e27); + IonPoolExposed(address(weEthIonPool)).setSupplyFactor(supplyFactor); + + uint256 supplyCap = bound(assets, 100e18, type(uint128).max); + weEthIonPool.updateSupplyCap(supplyCap); + + uint256 initialDeposit = bound(assets, 1e18, supplyCap - 10e18); + supply(address(this), weEthIonPool, initialDeposit); + uint256 initialTotalNormalized = weEthIonPool.totalSupplyUnaccrued(); + + uint256 supplyCapDiff = _zeroFloorSub(supplyCap, weEthIonPool.getTotalUnderlyingClaims()); + + // `IonPool.supply` math + uint256 amountScaled = supplyCapDiff.rayDivDown(supplyFactor); + uint256 resultingTotalNormalized = initialTotalNormalized + amountScaled; + + uint256 resultingTotalClaim = resultingTotalNormalized.rayMulDown(supplyFactor); + + supply(address(this), weEthIonPool, supplyCapDiff); + + assertEq( + resultingTotalClaim, weEthIonPool.getTotalUnderlyingClaims(), "resulting should be the same as calculated" + ); + + // Is it possible that depositing this supplyCapDiff results in a revert? + // `IonPool` compares `getTotalUnderlyingClaims > _supplyCap` + assertLe(resultingTotalClaim, supplyCap, "supply cap reached"); + assertLe(weEthIonPool.getTotalUnderlyingClaims(), supplyCap, "supply cap reached"); + } + + // Supplying the diff in the allocation cap should never end up violating + // the allocation cap. + // Is it possible that the `maxDeposit` returns more than the allocation cap? + function testFuzz_DepositToFillAllocationCap(uint256 assets, uint256 supplyFactor) public { + supplyFactor = bound(supplyFactor, 1e27, 10e27); + IonPoolExposed(address(weEthIonPool)).setSupplyFactor(supplyFactor); + + uint256 allocationCap = bound(assets, 100e18, type(uint128).max); + updateAllocationCaps(vault, allocationCap, type(uint128).max, 0); + + // Deposit, but leave some room below the allocation cap. + uint256 depositAmt = bound(assets, 1e18, allocationCap - 10e18); + setERC20Balance(address(BASE_ASSET), address(this), depositAmt); + vault.deposit(depositAmt, address(this)); + + uint256 initialTotalNormalized = weEthIonPool.totalSupplyUnaccrued(); + + uint256 allocationCapDiff = _zeroFloorSub(allocationCap, weEthIonPool.getUnderlyingClaimOf(address(vault))); + + uint256 amountScaled = allocationCapDiff.rayDivDown(supplyFactor); + uint256 resultingTotalNormalized = initialTotalNormalized + amountScaled; + uint256 resultingTotalClaim = resultingTotalNormalized.rayMulDown(supplyFactor); + + // Try to deposit a little more than the first allocation cap would + // allow, then check whether it's possible to violate the first + // allocation cap. + + setERC20Balance(address(BASE_ASSET), address(this), allocationCapDiff + 123e18); + vault.deposit(allocationCapDiff + 123e18, address(this)); + + uint256 actualTotalClaim = weEthIonPool.getUnderlyingClaimOf(address(vault)); + assertEq(resultingTotalClaim, actualTotalClaim, "expected and actual must be equal"); + + assertLe(resultingTotalClaim, allocationCap, "expected claim le to allocation cap"); + assertLe(actualTotalClaim, allocationCap, "actual claim le to allocation cap"); + } } contract VaultWithYieldAndFee_Fuzz is VaultSharedSetup { From 8bdb2098e9cffec7f516f43aadaa00ec239c527c Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Mon, 13 May 2024 14:29:35 -0400 Subject: [PATCH 14/20] fix: instead of early exit, set utilization rate to zero if totalEthSupply scaled by distribution factor truncates to zero --- src/InterestRate.sol | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/InterestRate.sol b/src/InterestRate.sol index d4c7c841..d2f84798 100644 --- a/src/InterestRate.sol +++ b/src/InterestRate.sol @@ -326,15 +326,12 @@ contract InterestRate { return (ilkData.minimumKinkRate, ilkData.reserveFactor.scaleUpToRay(4)); } - uint256 totalEthSupplyScaled = totalEthSupply.wadMulDown(distributionFactor.scaleUpToWad(4)); // If the `totalEthSupply` is small enough to truncate to zero, then - // simply return the minimum base rate. - if (totalEthSupplyScaled == 0) { - return (ilkData.minimumBaseRate, ilkData.reserveFactor.scaleUpToRay(4)); - } + // treat the utilization as zero. + uint256 totalEthSupplyScaled = totalEthSupply.wadMulDown(distributionFactor.scaleUpToWad(4)); // [RAD] / [WAD] = [RAY] - uint256 utilizationRate = totalEthSupply == 0 ? 0 : totalIlkDebt / totalEthSupplyScaled; + uint256 utilizationRate = totalEthSupplyScaled == 0 ? 0 : totalIlkDebt / totalEthSupplyScaled; // Avoid stack too deep uint256 adjustedBelowKinkSlope; From 31eaad79cfee4b9bd5b27da4f6e49173d7b409a1 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Sat, 11 May 2024 15:58:58 -0400 Subject: [PATCH 15/20] fix: if borrow rate is zero, only update the ilk last updated --- src/IonPool.sol | 10 +++---- test/unit/concrete/IonPool.t.sol | 46 ++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/src/IonPool.sol b/src/IonPool.sol index 97e53c2a..d4f859b8 100644 --- a/src/IonPool.sol +++ b/src/IonPool.sol @@ -490,14 +490,14 @@ contract IonPool is PausableUpgradeable, RewardToken { (uint256 borrowRate, uint256 reserveFactor) = $.interestRateModule.calculateInterestRate(ilkIndex, totalDebt, totalEthSupply); - if (borrowRate == 0) return (0, 0, 0, 0, 0); - - // Calculates borrowRate ^ (time) and returns the result with RAY precision - uint256 borrowRateExpT = _rpow(borrowRate + RAY, block.timestamp - ilk.lastRateUpdate, RAY); - // Unsafe cast OK timestampIncrease = uint48(block.timestamp) - ilk.lastRateUpdate; + if (borrowRate == 0) return (0, 0, 0, 0, timestampIncrease); + + // Calculates borrowRate ^ (time) and returns the result with RAY precision + uint256 borrowRateExpT = _rpow(borrowRate + RAY, timestampIncrease, RAY); + // Debt distribution // This form of rate accrual is much safer than distributing the new // debt increase to the total debt since low debt amounts won't cause diff --git a/test/unit/concrete/IonPool.t.sol b/test/unit/concrete/IonPool.t.sol index 0863f53a..fec70e9c 100644 --- a/test/unit/concrete/IonPool.t.sol +++ b/test/unit/concrete/IonPool.t.sol @@ -1129,6 +1129,52 @@ contract IonPool_InterestTest is IonPoolSharedSetup, IIonPoolEvents { } } + // If zero borrow rate, only the last updated timestamp should update + function test_CalculateRewardAndDebtDistributionZeroBorrowRate() external { + // update interest rate module to have zero rates. + IlkData[] memory ilkConfigs = new IlkData[](3); + uint16[] memory distributionFactors = new uint16[](3); + distributionFactors[0] = 0.2e4; + distributionFactors[1] = 0.4e4; + distributionFactors[2] = 0.4e4; + + for (uint8 i; i != 3; ++i) { + IlkData memory ilkConfig = IlkData({ + adjustedProfitMargin: 0, + minimumKinkRate: 0, + reserveFactor: 0, + adjustedBaseRate: 0, + minimumBaseRate: 0, + optimalUtilizationRate: 9000, + distributionFactor: distributionFactors[i], + adjustedAboveKinkSlope: 0, + minimumAboveKinkSlope: 0 + }); + ilkConfigs[i] = ilkConfig; + } + + interestRateModule = new InterestRate(ilkConfigs, apyOracle); + ionPool.updateInterestRateModule(interestRateModule); + + vm.warp(block.timestamp + 1 days); + + ( + uint256 totalSupplyFactorIncrease, + , + uint104[] memory rateIncreases, + uint256 totalDebtIncrease, + uint48[] memory timestampIncreases + ) = ionPool.calculateRewardAndDebtDistribution(); + + assertEq(totalSupplyFactorIncrease, 0, "total supply factor"); + assertEq(totalDebtIncrease, 0, "total debt increase"); + + for (uint8 i; i != 3; ++i) { + assertEq(rateIncreases[i], 0, "rate"); + assertEq(timestampIncreases[i], 1 days, "timestamp increase"); + } + } + function test_AccrueInterest() public { uint256 collateralDepositAmount = 10e18; uint256 normalizedBorrowAmount = 5e18; From 597dbf9b2ad5fed25fa71de091b155b14eb57251 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Wed, 15 May 2024 02:16:05 -0400 Subject: [PATCH 16/20] fix: handle interest accrual while paused in calculateRewardAndDebtDistributionForIlk --- src/IonPool.sol | 5 ++- src/vault/Vault.sol | 11 +---- test/unit/concrete/IonPool.t.sol | 61 ++++++++++++++++++++++++++++ test/unit/concrete/vault/Vault.t.sol | 45 ++++++++++++++++---- test/unit/fuzz/vault/Vault.t.sol | 5 ++- 5 files changed, 106 insertions(+), 21 deletions(-) diff --git a/src/IonPool.sol b/src/IonPool.sol index 233c48ca..4bdf9d41 100644 --- a/src/IonPool.sol +++ b/src/IonPool.sol @@ -478,7 +478,10 @@ contract IonPool is PausableUpgradeable, RewardToken { Ilk storage ilk = $.ilks[ilkIndex]; uint256 _totalNormalizedDebt = ilk.totalNormalizedDebt; - if (_totalNormalizedDebt == 0 || block.timestamp == ilk.lastRateUpdate) { + // Because all interest that would have accrued during a pause is + // cancelled upon `unpause`, we return zero interest while markets are + // paused. + if (_totalNormalizedDebt == 0 || block.timestamp == ilk.lastRateUpdate || paused()) { // Unsafe cast OK // block.timestamp - ilk.lastRateUpdate will almost always be 0 // here. The exception is on first borrow. diff --git a/src/vault/Vault.sol b/src/vault/Vault.sol index 0b699cfa..6b42ebc7 100644 --- a/src/vault/Vault.sol +++ b/src/vault/Vault.sol @@ -655,16 +655,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy for (uint256 i; i != _supportedMarketsLength;) { IIonPool pool = IIonPool(supportedMarkets.at(i)); - uint256 assetsInPool; - if (pool == IDLE) { - assetsInPool = BASE_ASSET.balanceOf(address(this)); - } else { - if (pool.paused()) { - assetsInPool = pool.balanceOfUnaccrued(address(this)); - } else { - assetsInPool = pool.balanceOf(address(this)); - } - } + uint256 assetsInPool = pool == IDLE ? BASE_ASSET.balanceOf(address(this)) : pool.balanceOf(address(this)); assets += assetsInPool; diff --git a/test/unit/concrete/IonPool.t.sol b/test/unit/concrete/IonPool.t.sol index 0863f53a..96a3e2ad 100644 --- a/test/unit/concrete/IonPool.t.sol +++ b/test/unit/concrete/IonPool.t.sol @@ -1192,6 +1192,67 @@ contract IonPool_InterestTest is IonPoolSharedSetup, IIonPoolEvents { previousRates[i] = rate; } } + + function test_AccrueInterestWhenPaused() public { + uint256 collateralDepositAmount = 10e18; + uint256 normalizedBorrowAmount = 5e18; + + for (uint8 i = 0; i < lens.ilkCount(iIonPool); i++) { + vm.prank(borrower1); + ionPool.depositCollateral(i, borrower1, borrower1, collateralDepositAmount, new bytes32[](0)); + + uint256 rate = ionPool.rate(i); + uint256 liquidityBefore = lens.liquidity(iIonPool); + + assertEq(ionPool.collateral(i, borrower1), collateralDepositAmount); + assertEq(underlying.balanceOf(borrower1), normalizedBorrowAmount.rayMulDown(rate) * i); + + vm.prank(borrower1); + ionPool.borrow(i, borrower1, borrower1, normalizedBorrowAmount, new bytes32[](0)); + + uint256 liquidityRemoved = normalizedBorrowAmount.rayMulDown(rate); + + assertEq(ionPool.normalizedDebt(i, borrower1), normalizedBorrowAmount); + assertEq(lens.totalNormalizedDebt(iIonPool, i), normalizedBorrowAmount); + assertEq(lens.liquidity(iIonPool), liquidityBefore - liquidityRemoved); + assertEq(underlying.balanceOf(borrower1), normalizedBorrowAmount.rayMulDown(rate) * (i + 1)); + } + + vm.warp(block.timestamp + 1 hours); + + ionPool.pause(); + + uint256 rate0AfterPause = ionPool.rate(0); + uint256 rate1AfterPause = ionPool.rate(1); + uint256 rate2AfterPause = ionPool.rate(2); + + uint256 supplyFactorAfterPause = ionPool.supplyFactor(); + uint256 lenderBalanceAfterPause = ionPool.balanceOf(lender2); + + vm.warp(block.timestamp + 365 days); + + ( + uint256 totalSupplyFactorIncrease, + uint256 treasuryMintAmount, + uint104[] memory rateIncreases, + uint256 totalDebtIncrease, + uint48[] memory timestampIncreases + ) = ionPool.calculateRewardAndDebtDistribution(); + + assertEq(totalSupplyFactorIncrease, 0, "no supply factor increase"); + assertEq(treasuryMintAmount, 0, "no treasury mint amount"); + for (uint8 i = 0; i < lens.ilkCount(iIonPool); i++) { + assertEq(rateIncreases[i], 0, "no rate increase"); + assertEq(timestampIncreases[i], 365 days, "no timestamp increase"); + } + assertEq(totalDebtIncrease, 0, "no total debt increase"); + + assertEq(ionPool.balanceOf(lender2), lenderBalanceAfterPause, "lender balance doesn't change"); + assertEq(ionPool.supplyFactor(), supplyFactorAfterPause, "supply factor doesn't change"); + assertEq(ionPool.rate(0), rate0AfterPause, "rate 0 doesn't change"); + assertEq(ionPool.rate(1), rate1AfterPause, "rate 1 doesn't change"); + assertEq(ionPool.rate(2), rate2AfterPause, "rate 2 doesn't change"); + } } contract IonPool_AdminTest is IonPoolSharedSetup { diff --git a/test/unit/concrete/vault/Vault.t.sol b/test/unit/concrete/vault/Vault.t.sol index 60c3168a..fda64644 100644 --- a/test/unit/concrete/vault/Vault.t.sol +++ b/test/unit/concrete/vault/Vault.t.sol @@ -1445,21 +1445,35 @@ contract VaultERC4626ExternalViews is VaultSharedSetup { vault.deposit(depositAmt, address(this)); assertEq(weEthIonPool.balanceOf(address(vault)), depositAmt, "weEthIonPool balance"); - // pause the weEthIonPool + + // Pause the weEthIonPool, stop accruing interest weEthIonPool.pause(); + assertTrue(weEthIonPool.paused(), "weEthIonPool is paused"); vm.warp(block.timestamp + 365 days); - assertGt(weEthIonPool.balanceOf(address(vault)), depositAmt, "weEthIonPool accrues interest"); - assertLt( + assertEq(weEthIonPool.balanceOf(address(vault)), depositAmt, "weEthIonPool accrues interest"); + assertEq( weEthIonPool.balanceOfUnaccrued(address(vault)), weEthIonPool.balanceOf(address(vault)), "weEthIonPool unaccrued balance" ); - assertTrue(weEthIonPool.paused(), "weEthIonPool is paused"); uint256 totalAssets = vault.totalAssets(); assertEq(totalAssets, depositAmt, "total assets with paused IonPool does not include interest"); + + // When unpaused, should now accrue interest + weEthIonPool.unpause(); + vm.warp(block.timestamp + 365 days); + + assertGt(weEthIonPool.balanceOf(address(vault)), depositAmt, "weEthIonPool accrues interest"); + assertGt( + weEthIonPool.balanceOf(address(vault)), + weEthIonPool.balanceOfUnaccrued(address(vault)), + "weEthIonPool unaccrued balance" + ); + + assertGt(vault.totalAssets(), depositAmt, "total assets with paused IonPool does not include interest"); } function test_TotalAssetsWithMultiplePausedIonPools() public { @@ -1511,15 +1525,30 @@ contract VaultERC4626ExternalViews is VaultSharedSetup { vm.warp(block.timestamp + 365 days); - assertGt(weEthIonPool.balanceOf(address(vault)), weEthIonPoolAmt, "weEthIonPool balance increases"); - assertGt(rsEthIonPool.balanceOf(address(vault)), rsEthIonPoolAmt, "rsEthIonPool balance does not change"); - assertGt(rswEthIonPool.balanceOf(address(vault)), rswEthIonPoolAmt, "rswEthIonPool balance increases"); - // The 'unaccrued' values should not change assertEq(weEthIonPool.balanceOfUnaccrued(address(vault)), weEthIonPoolAmt, "weEthIonPool balance"); assertEq(rsEthIonPool.balanceOfUnaccrued(address(vault)), rsEthIonPoolAmt, "rsEthIonPool balance"); assertEq(rswEthIonPool.balanceOfUnaccrued(address(vault)), rswEthIonPoolAmt, "rswEthIonPool balance"); + // When paused, the unaccrued and accrued balanceOf should be the same + assertEq( + weEthIonPool.balanceOf(address(vault)), + weEthIonPool.balanceOfUnaccrued(address(vault)), + "weEthIonPool balance increases" + ); + assertEq( + rswEthIonPool.balanceOf(address(vault)), + rswEthIonPool.balanceOfUnaccrued(address(vault)), + "rswEthIonPool balance increases" + ); + + // When not paused, the accrued balanceOf should be greater + assertGt( + rsEthIonPool.balanceOf(address(vault)), + rsEthIonPool.balanceOfUnaccrued(address(vault)), + "rsEthIonPool balance does not change" + ); + uint256 expectedTotalAssets = weEthIonPool.balanceOfUnaccrued(address(vault)) + rsEthIonPool.balanceOf(address(vault)) + rswEthIonPool.balanceOfUnaccrued(address(vault)); diff --git a/test/unit/fuzz/vault/Vault.t.sol b/test/unit/fuzz/vault/Vault.t.sol index 32cc524f..ef5c6767 100644 --- a/test/unit/fuzz/vault/Vault.t.sol +++ b/test/unit/fuzz/vault/Vault.t.sol @@ -124,8 +124,9 @@ contract VaultWithYieldAndFee_Fuzz is VaultSharedSetup { // expected resulting state uint256 expectedFeeAssets = interestAccrued.mulDiv(feePerc, RAY); - uint256 expectedFeeShares = - expectedFeeAssets.mulDiv(vault.totalSupply(), newTotalAssets - expectedFeeAssets, Math.Rounding.Floor); + uint256 expectedFeeShares = expectedFeeAssets.mulDiv( + vault.totalSupply() + 1, newTotalAssets - expectedFeeAssets + 1, Math.Rounding.Floor + ); uint256 expectedUserAssets = prevUserAssets + interestAccrued.mulDiv(RAY - feePerc, RAY); From e4637777e372e9b9a84026fdd2c514daf018e968 Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Mon, 13 May 2024 03:36:30 -0400 Subject: [PATCH 17/20] feat: to further defend against inflation attacks on top of virtual assets and shares, the factory enforces initial deposit into the vault at deployment time --- src/vault/Vault.sol | 62 +++++-- src/vault/VaultFactory.sol | 21 ++- test/fork/concrete/vault/VaultFactory.t.sol | 172 ++++++++++++++++++-- test/helpers/VaultSharedSetup.sol | 37 +++-- test/unit/concrete/vault/Vault.t.sol | 12 +- test/unit/fuzz/vault/Vault.t.sol | 5 +- 6 files changed, 253 insertions(+), 56 deletions(-) diff --git a/src/vault/Vault.sol b/src/vault/Vault.sol index 6b42ebc7..a5360410 100644 --- a/src/vault/Vault.sol +++ b/src/vault/Vault.sol @@ -86,6 +86,13 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy int256 assets; } + struct MarketsArgs { + IIonPool[] marketsToAdd; + uint256[] allocationCaps; + IIonPool[] newSupplyQueue; + IIonPool[] newWithdrawQueue; + } + constructor( IERC20 _baseAsset, address _feeRecipient, @@ -93,7 +100,8 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy string memory _name, string memory _symbol, uint48 initialDelay, - address initialDefaultAdmin + address initialDefaultAdmin, + MarketsArgs memory marketsArgs ) ERC4626(_baseAsset) ERC20(_name, _symbol) @@ -105,6 +113,13 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy feeRecipient = _feeRecipient; DECIMALS_OFFSET = uint8(_zeroFloorSub(uint256(18), IERC20Metadata(address(_baseAsset)).decimals())); + + _addSupportedMarkets( + marketsArgs.marketsToAdd, + marketsArgs.allocationCaps, + marketsArgs.newSupplyQueue, + marketsArgs.newWithdrawQueue + ); } /** @@ -138,13 +153,24 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy * @param newWithdrawQueue Desired withdraw queue of IonPools for all resulting supported markets. */ function addSupportedMarkets( - IIonPool[] calldata marketsToAdd, - uint256[] calldata allocationCaps, - IIonPool[] calldata newSupplyQueue, - IIonPool[] calldata newWithdrawQueue + IIonPool[] memory marketsToAdd, + uint256[] memory allocationCaps, + IIonPool[] memory newSupplyQueue, + IIonPool[] memory newWithdrawQueue ) - public + external onlyRole(OWNER_ROLE) + { + _addSupportedMarkets(marketsToAdd, allocationCaps, newSupplyQueue, newWithdrawQueue); + } + + function _addSupportedMarkets( + IIonPool[] memory marketsToAdd, + uint256[] memory allocationCaps, + IIonPool[] memory newSupplyQueue, + IIonPool[] memory newWithdrawQueue + ) + internal { if (marketsToAdd.length != allocationCaps.length) revert MarketsAndAllocationCapLengthMustBeEqual(); @@ -167,8 +193,8 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy } } - updateSupplyQueue(newSupplyQueue); - updateWithdrawQueue(newWithdrawQueue); + _updateSupplyQueue(newSupplyQueue); + _updateWithdrawQueue(newWithdrawQueue); } /** @@ -211,8 +237,8 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy ++i; } } - updateSupplyQueue(newSupplyQueue); - updateWithdrawQueue(newWithdrawQueue); + _updateSupplyQueue(newSupplyQueue); + _updateWithdrawQueue(newWithdrawQueue); } /** @@ -220,7 +246,11 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy * @dev Each IonPool in the queue must be part of the `supportedMarkets` set. * @param newSupplyQueue The new supply queue ordering. */ - function updateSupplyQueue(IIonPool[] calldata newSupplyQueue) public onlyRole(ALLOCATOR_ROLE) { + function updateSupplyQueue(IIonPool[] memory newSupplyQueue) external onlyRole(ALLOCATOR_ROLE) { + _updateSupplyQueue(newSupplyQueue); + } + + function _updateSupplyQueue(IIonPool[] memory newSupplyQueue) internal { _validateQueueInput(newSupplyQueue); supplyQueue = newSupplyQueue; @@ -233,7 +263,11 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy * @dev The IonPool in the queue must be part of the `supportedMarkets` set. * @param newWithdrawQueue The new withdraw queue ordering. */ - function updateWithdrawQueue(IIonPool[] calldata newWithdrawQueue) public onlyRole(ALLOCATOR_ROLE) { + function updateWithdrawQueue(IIonPool[] memory newWithdrawQueue) external onlyRole(ALLOCATOR_ROLE) { + _updateWithdrawQueue(newWithdrawQueue); + } + + function _updateWithdrawQueue(IIonPool[] memory newWithdrawQueue) internal { _validateQueueInput(newWithdrawQueue); withdrawQueue = newWithdrawQueue; @@ -249,7 +283,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy * The above rule enforces that the queue must have all and only the elements in the `supportedMarkets` set. * @param queue The queue being validated. */ - function _validateQueueInput(IIonPool[] calldata queue) internal view { + function _validateQueueInput(IIonPool[] memory queue) internal view { uint256 _supportedMarketsLength = supportedMarkets.length(); uint256 queueLength = queue.length; @@ -705,9 +739,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy function _deposit(address caller, address receiver, uint256 assets, uint256 shares) internal override { super._deposit(caller, receiver, assets, shares); - _supplyToIonPool(assets); - _updateLastTotalAssets(lastTotalAssets + assets); } diff --git a/src/vault/VaultFactory.sol b/src/vault/VaultFactory.sol index 4bee1097..9c5d2df2 100644 --- a/src/vault/VaultFactory.sol +++ b/src/vault/VaultFactory.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.21; import { Vault } from "./Vault.sol"; import { IERC20 } from "openzeppelin-contracts/contracts/interfaces/IERC20.sol"; +import { SafeERC20 } from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol"; /** * @title Ion Lending Vault Factory @@ -10,6 +11,8 @@ import { IERC20 } from "openzeppelin-contracts/contracts/interfaces/IERC20.sol"; * @notice Factory contract for deploying Ion Lending Vaults. */ contract VaultFactory { + using SafeERC20 for IERC20; + // --- Events --- event CreateVault( @@ -25,7 +28,10 @@ contract VaultFactory { // --- External --- /** - * @notice Deploys a new Ion Lending Vault. + * @notice Deploys a new Ion Lending Vault. Transfers 1 gwei of base asset + * from the caller to initiate an initial deposit to the vault. + * @dev The 1e9 initial deposit amount was chosen to defend against + * inflation attacks. * @param baseAsset The asset that is being lent out to IonPools. * @param feeRecipient Address that receives the accrued manager fees. * @param feePercentage Fee percentage to be set. @@ -34,6 +40,8 @@ contract VaultFactory { * @param initialDelay The initial delay for default admin transfers. * @param initialDefaultAdmin The initial default admin for the vault. * @param salt The salt used for CREATE2 deployment. + * @param marketsArgs Arguments for the markets to be added to the vault. + * @param initialDeposit The initial deposit to be made to the vault. */ function createVault( IERC20 baseAsset, @@ -43,16 +51,21 @@ contract VaultFactory { string memory symbol, uint48 initialDelay, address initialDefaultAdmin, - bytes32 salt + bytes32 salt, + Vault.MarketsArgs memory marketsArgs, + uint256 initialDeposit ) external returns (Vault vault) { - // TODO use named args syntax vault = new Vault{ salt: salt }( - baseAsset, feeRecipient, feePercentage, name, symbol, initialDelay, initialDefaultAdmin + baseAsset, feeRecipient, feePercentage, name, symbol, initialDelay, initialDefaultAdmin, marketsArgs ); + baseAsset.safeTransferFrom(msg.sender, address(this), initialDeposit); + baseAsset.approve(address(vault), initialDeposit); + vault.deposit(initialDeposit, msg.sender); + emit CreateVault(address(vault), baseAsset, feeRecipient, feePercentage, name, symbol, initialDefaultAdmin); } } diff --git a/test/fork/concrete/vault/VaultFactory.t.sol b/test/fork/concrete/vault/VaultFactory.t.sol index 1715b1a3..2309f36c 100644 --- a/test/fork/concrete/vault/VaultFactory.t.sol +++ b/test/fork/concrete/vault/VaultFactory.t.sol @@ -6,6 +6,7 @@ import { VaultFactory } from "./../../../../src/vault/VaultFactory.sol"; import { VaultSharedSetup } from "../../../helpers/VaultSharedSetup.sol"; import { ERC20PresetMinterPauser } from "../../../helpers/ERC20PresetMinterPauser.sol"; import { IERC20 } from "@openzeppelin/contracts/interfaces/IERC20.sol"; +import { IIonPool } from "./../../../../src/interfaces/IIonPool.sol"; contract VaultFactoryTest is VaultSharedSetup { VaultFactory factory; @@ -16,30 +17,116 @@ contract VaultFactoryTest is VaultSharedSetup { string internal name = "Vault Token"; string internal symbol = "VT"; + IIonPool[] internal marketsToAdd; + uint256[] internal allocationCaps; + IIonPool[] internal newSupplyQueue; + IIonPool[] internal newWithdrawQueue; + function setUp() public override { super.setUp(); factory = new VaultFactory(); + + marketsToAdd.push(weEthIonPool); + marketsToAdd.push(rsEthIonPool); + marketsToAdd.push(rswEthIonPool); + + allocationCaps.push(1e18); + allocationCaps.push(2e18); + allocationCaps.push(3e18); + + newSupplyQueue.push(weEthIonPool); + newSupplyQueue.push(rswEthIonPool); + newSupplyQueue.push(rsEthIonPool); + + newWithdrawQueue.push(rswEthIonPool); + newWithdrawQueue.push(rsEthIonPool); + newWithdrawQueue.push(weEthIonPool); + + marketsArgs.marketsToAdd = marketsToAdd; + marketsArgs.allocationCaps = allocationCaps; + marketsArgs.newSupplyQueue = newSupplyQueue; + marketsArgs.newWithdrawQueue = newWithdrawQueue; + + setERC20Balance(address(BASE_ASSET), address(this), MIN_INITIAL_DEPOSIT); + BASE_ASSET.approve(address(factory), MIN_INITIAL_DEPOSIT); } function test_CreateVault() public { bytes32 salt = keccak256("random salt"); - Vault vault = - factory.createVault(baseAsset, feeRecipient, feePercentage, name, symbol, INITIAL_DELAY, VAULT_ADMIN, salt); + Vault vault = factory.createVault( + baseAsset, + feeRecipient, + feePercentage, + name, + symbol, + INITIAL_DELAY, + VAULT_ADMIN, + salt, + marketsArgs, + MIN_INITIAL_DEPOSIT + ); - assertEq(VAULT_ADMIN, vault.defaultAdmin(), "default admin"); - assertEq(feeRecipient, vault.feeRecipient(), "fee recipient"); - assertEq(address(baseAsset), address(vault.BASE_ASSET()), "base asset"); + address[] memory supportedMarkets = vault.getSupportedMarkets(); + + IIonPool firstInSupplyQueue = vault.supplyQueue(0); + IIonPool secondInSupplyQueue = vault.supplyQueue(1); + IIonPool thirdInSupplyQueue = vault.supplyQueue(2); + + IIonPool firstInWithdrawQueue = vault.withdrawQueue(0); + IIonPool secondInWithdrawQueue = vault.withdrawQueue(1); + IIonPool thirdInWithdrawQueue = vault.withdrawQueue(2); + + assertEq(vault.defaultAdmin(), VAULT_ADMIN, "default admin"); + assertEq(vault.feeRecipient(), feeRecipient, "fee recipient"); + assertEq(vault.feePercentage(), feePercentage, "fee percentage"); + assertEq(address(vault.BASE_ASSET()), address(baseAsset), "base asset"); + + assertEq(supportedMarkets.length, 3, "supported markets length"); + + for (uint256 i = 0; i != supportedMarkets.length; ++i) { + assertEq(address(supportedMarkets[i]), address(marketsToAdd[i]), "supported markets"); + assertEq(address(vault.supplyQueue(i)), address(newSupplyQueue[i]), "supply queue"); + assertEq(address(vault.withdrawQueue(i)), address(newWithdrawQueue[i]), "withdraw queue"); + } + + // initial deposits + assertEq(BASE_ASSET.balanceOf(address(this)), 0, "initial deposit spent"); + assertEq(vault.totalAssets(), MIN_INITIAL_DEPOSIT, "total assets"); + assertEq(vault.totalSupply(), MIN_INITIAL_DEPOSIT, "total supply"); } function test_CreateVault_Twice() public { bytes32 salt = keccak256("first random salt"); - Vault vault = - factory.createVault(baseAsset, feeRecipient, feePercentage, name, symbol, INITIAL_DELAY, VAULT_ADMIN, salt); + Vault vault = factory.createVault( + baseAsset, + feeRecipient, + feePercentage, + name, + symbol, + INITIAL_DELAY, + VAULT_ADMIN, + salt, + marketsArgs, + MIN_INITIAL_DEPOSIT + ); + + setERC20Balance(address(BASE_ASSET), address(this), MIN_INITIAL_DEPOSIT); + BASE_ASSET.approve(address(factory), MIN_INITIAL_DEPOSIT); bytes32 salt2 = keccak256("second random salt"); - Vault vault2 = - factory.createVault(baseAsset, feeRecipient, feePercentage, name, symbol, INITIAL_DELAY, VAULT_ADMIN, salt2); + Vault vault2 = factory.createVault( + baseAsset, + feeRecipient, + feePercentage, + name, + symbol, + INITIAL_DELAY, + VAULT_ADMIN, + salt2, + marketsArgs, + MIN_INITIAL_DEPOSIT + ); assertEq(VAULT_ADMIN, vault.defaultAdmin(), "default admin"); assertEq(feeRecipient, vault.feeRecipient(), "fee recipient"); @@ -52,24 +139,77 @@ contract VaultFactoryTest is VaultSharedSetup { function test_Revert_CreateVault_SameSaltTwice() public { bytes32 salt = keccak256("random salt"); - Vault vault = - factory.createVault(baseAsset, feeRecipient, feePercentage, name, symbol, INITIAL_DELAY, VAULT_ADMIN, salt); + Vault vault = factory.createVault( + baseAsset, + feeRecipient, + feePercentage, + name, + symbol, + INITIAL_DELAY, + VAULT_ADMIN, + salt, + marketsArgs, + MIN_INITIAL_DEPOSIT + ); vm.expectRevert(); - Vault vault2 = - factory.createVault(baseAsset, feeRecipient, feePercentage, name, symbol, INITIAL_DELAY, VAULT_ADMIN, salt); + Vault vault2 = factory.createVault( + baseAsset, + feeRecipient, + feePercentage, + name, + symbol, + INITIAL_DELAY, + VAULT_ADMIN, + salt, + marketsArgs, + MIN_INITIAL_DEPOSIT + ); } function test_CreateVault_SameSaltDifferentBytecode() public { bytes32 salt = keccak256("random salt"); - Vault vault = - factory.createVault(BASE_ASSET, feeRecipient, feePercentage, name, symbol, INITIAL_DELAY, VAULT_ADMIN, salt); + Vault vault = factory.createVault( + BASE_ASSET, + feeRecipient, + feePercentage, + name, + symbol, + INITIAL_DELAY, + VAULT_ADMIN, + salt, + marketsArgs, + MIN_INITIAL_DEPOSIT + ); + // Deploy a vault with different base assets IERC20 diffBaseAsset = IERC20(address(new ERC20PresetMinterPauser("Another Wrapped Staked ETH", "wstETH2"))); + IIonPool[] memory markets = new IIonPool[](3); + markets[0] = deployIonPool(diffBaseAsset, WEETH, address(this)); + markets[1] = deployIonPool(diffBaseAsset, RSETH, address(this)); + markets[2] = deployIonPool(diffBaseAsset, RSWETH, address(this)); + + marketsArgs.marketsToAdd = markets; + marketsArgs.allocationCaps = allocationCaps; + marketsArgs.newSupplyQueue = markets; + marketsArgs.newWithdrawQueue = markets; + + setERC20Balance(address(diffBaseAsset), address(this), MIN_INITIAL_DEPOSIT); + diffBaseAsset.approve(address(factory), MIN_INITIAL_DEPOSIT); + Vault vault2 = factory.createVault( - diffBaseAsset, feeRecipient, feePercentage, name, symbol, INITIAL_DELAY, VAULT_ADMIN, salt + diffBaseAsset, + feeRecipient, + feePercentage, + name, + symbol, + INITIAL_DELAY, + VAULT_ADMIN, + salt, + marketsArgs, + MIN_INITIAL_DEPOSIT ); require(address(vault) != address(vault2), "different deployment address"); diff --git a/test/helpers/VaultSharedSetup.sol b/test/helpers/VaultSharedSetup.sol index 39618c79..84650c63 100644 --- a/test/helpers/VaultSharedSetup.sol +++ b/test/helpers/VaultSharedSetup.sol @@ -32,6 +32,8 @@ contract VaultSharedSetup is IonPoolSharedSetup { StdStorage stdstore1; Vault vault; + Vault.MarketsArgs marketsArgs; + Vault.MarketsArgs emptyMarketsArgs; // roles address constant VAULT_ADMIN = address(uint160(uint256(keccak256("VAULT_ADMIN")))); @@ -42,6 +44,10 @@ contract VaultSharedSetup is IonPoolSharedSetup { address constant FEE_RECIPIENT = address(uint160(uint256(keccak256("FEE_RECIPIENT")))); uint256 constant ZERO_FEES = 0; + uint256 constant MIN_INITIAL_DEPOSIT = 1e9; + + bytes32 constant SALT = keccak256("SALT"); + IERC20 immutable BASE_ASSET = IERC20(address(new ERC20PresetMinterPauser("Lido Wrapped Staked ETH", "wstETH"))); IERC20 immutable WEETH = IERC20(address(new ERC20PresetMinterPauser("EtherFi Restaked ETH", "weETH"))); IERC20 immutable RSETH = IERC20(address(new ERC20PresetMinterPauser("KelpDAO Restaked ETH", "rsETH"))); @@ -69,31 +75,30 @@ contract VaultSharedSetup is IonPoolSharedSetup { rsEthIonPool = deployIonPool(BASE_ASSET, RSETH, address(this)); rswEthIonPool = deployIonPool(BASE_ASSET, RSWETH, address(this)); - vault = new Vault(BASE_ASSET, FEE_RECIPIENT, ZERO_FEES, "Ion Vault Token", "IVT", INITIAL_DELAY, VAULT_ADMIN); - - vm.startPrank(vault.defaultAdmin()); - - vault.grantRole(vault.OWNER_ROLE(), OWNER); - vault.grantRole(vault.ALLOCATOR_ROLE(), OWNER); // OWNER also needs to be ALLOCATOR in order to update queues - // inside `addSupportedMarkets`. - vault.grantRole(vault.ALLOCATOR_ROLE(), ALLOCATOR); - markets = new IIonPool[](3); markets[0] = weEthIonPool; markets[1] = rsEthIonPool; markets[2] = rswEthIonPool; - vm.stopPrank(); + marketsArgs.marketsToAdd = markets; + marketsArgs.allocationCaps = ZERO_ALLO_CAPS; + marketsArgs.newSupplyQueue = markets; + marketsArgs.newWithdrawQueue = markets; - vm.prank(OWNER); - vault.addSupportedMarkets(markets, ZERO_ALLO_CAPS, markets, markets); + vault = new Vault{ salt: SALT }( + BASE_ASSET, FEE_RECIPIENT, ZERO_FEES, "Ion Vault Token", "IVT", INITIAL_DELAY, VAULT_ADMIN, marketsArgs + ); BASE_ASSET.approve(address(vault), type(uint256).max); - // pools = new IIonPool[](3); - // pools[0] = weEthIonPool; - // pools[1] = rsEthIonPool; - // pools[2] = rswEthIonPool; + vm.startPrank(vault.defaultAdmin()); + + vault.grantRole(vault.OWNER_ROLE(), OWNER); + vault.grantRole(vault.ALLOCATOR_ROLE(), OWNER); // OWNER also needs to be ALLOCATOR in order to update queues + // inside `addSupportedMarkets`. + vault.grantRole(vault.ALLOCATOR_ROLE(), ALLOCATOR); + + vm.stopPrank(); weEthGemJoin = new GemJoin(IonPool(address(weEthIonPool)), IERC20(weEthIonPool.getIlkAddress(0)), 0, address(this)); diff --git a/test/unit/concrete/vault/Vault.t.sol b/test/unit/concrete/vault/Vault.t.sol index 31ad2d14..9039e0bb 100644 --- a/test/unit/concrete/vault/Vault.t.sol +++ b/test/unit/concrete/vault/Vault.t.sol @@ -25,7 +25,9 @@ contract VaultSetUpTest is VaultSharedSetup { } function test_AddSupportedMarketsSeparately() public { - vault = new Vault(BASE_ASSET, FEE_RECIPIENT, ZERO_FEES, "Ion Vault Token", "IVT", INITIAL_DELAY, VAULT_ADMIN); + vault = new Vault( + BASE_ASSET, FEE_RECIPIENT, ZERO_FEES, "Ion Vault Token", "IVT", INITIAL_DELAY, VAULT_ADMIN, emptyMarketsArgs + ); vm.startPrank(vault.defaultAdmin()); vault.grantRole(vault.OWNER_ROLE(), OWNER); @@ -87,7 +89,9 @@ contract VaultSetUpTest is VaultSharedSetup { } function test_AddSupportedMarketsTogether() public { - vault = new Vault(BASE_ASSET, FEE_RECIPIENT, ZERO_FEES, "Ion Vault Token", "IVT", INITIAL_DELAY, VAULT_ADMIN); + vault = new Vault( + BASE_ASSET, FEE_RECIPIENT, ZERO_FEES, "Ion Vault Token", "IVT", INITIAL_DELAY, VAULT_ADMIN, emptyMarketsArgs + ); vm.startPrank(vault.defaultAdmin()); vault.grantRole(vault.OWNER_ROLE(), OWNER); @@ -1128,7 +1132,9 @@ abstract contract VaultWithIdlePool is VaultSharedSetup { function setUp() public virtual override { super.setUp(); - vault = new Vault(BASE_ASSET, FEE_RECIPIENT, ZERO_FEES, "Ion Vault Token", "IVT", INITIAL_DELAY, VAULT_ADMIN); + vault = new Vault( + BASE_ASSET, FEE_RECIPIENT, ZERO_FEES, "Ion Vault Token", "IVT", INITIAL_DELAY, VAULT_ADMIN, emptyMarketsArgs + ); BASE_ASSET.approve(address(vault), type(uint256).max); diff --git a/test/unit/fuzz/vault/Vault.t.sol b/test/unit/fuzz/vault/Vault.t.sol index 32cc524f..ef5c6767 100644 --- a/test/unit/fuzz/vault/Vault.t.sol +++ b/test/unit/fuzz/vault/Vault.t.sol @@ -124,8 +124,9 @@ contract VaultWithYieldAndFee_Fuzz is VaultSharedSetup { // expected resulting state uint256 expectedFeeAssets = interestAccrued.mulDiv(feePerc, RAY); - uint256 expectedFeeShares = - expectedFeeAssets.mulDiv(vault.totalSupply(), newTotalAssets - expectedFeeAssets, Math.Rounding.Floor); + uint256 expectedFeeShares = expectedFeeAssets.mulDiv( + vault.totalSupply() + 1, newTotalAssets - expectedFeeAssets + 1, Math.Rounding.Floor + ); uint256 expectedUserAssets = prevUserAssets + interestAccrued.mulDiv(RAY - feePerc, RAY); From 72a1354eefa56baac06c7980d4cd68d64b3a166f Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Mon, 13 May 2024 22:36:30 -0400 Subject: [PATCH 18/20] feat: vault factory locks 1e3 shares minted by the deployer --- src/vault/VaultFactory.sol | 6 +- test/fork/concrete/vault/VaultFactory.t.sol | 103 +++++++++++++++++ test/unit/fuzz/vault/Vault.t.sol | 118 +++++++++++++++++++- 3 files changed, 224 insertions(+), 3 deletions(-) diff --git a/src/vault/VaultFactory.sol b/src/vault/VaultFactory.sol index 9c5d2df2..744da428 100644 --- a/src/vault/VaultFactory.sol +++ b/src/vault/VaultFactory.sol @@ -64,7 +64,11 @@ contract VaultFactory { baseAsset.safeTransferFrom(msg.sender, address(this), initialDeposit); baseAsset.approve(address(vault), initialDeposit); - vault.deposit(initialDeposit, msg.sender); + uint256 sharesMinted = vault.deposit(initialDeposit, address(this)); + + // The factory keeps 1e3 shares to reduce inflation attack vector. + // Effectively burns this amount of shares by locking it in the factory. + vault.transfer(msg.sender, sharesMinted - 1e3); emit CreateVault(address(vault), baseAsset, feeRecipient, feePercentage, name, symbol, initialDefaultAdmin); } diff --git a/test/fork/concrete/vault/VaultFactory.t.sol b/test/fork/concrete/vault/VaultFactory.t.sol index 2309f36c..9e59f349 100644 --- a/test/fork/concrete/vault/VaultFactory.t.sol +++ b/test/fork/concrete/vault/VaultFactory.t.sol @@ -8,6 +8,8 @@ import { ERC20PresetMinterPauser } from "../../../helpers/ERC20PresetMinterPause import { IERC20 } from "@openzeppelin/contracts/interfaces/IERC20.sol"; import { IIonPool } from "./../../../../src/interfaces/IIonPool.sol"; +import { console2 } from "forge-std/console2.sol"; + contract VaultFactoryTest is VaultSharedSetup { VaultFactory factory; @@ -94,6 +96,9 @@ contract VaultFactoryTest is VaultSharedSetup { assertEq(BASE_ASSET.balanceOf(address(this)), 0, "initial deposit spent"); assertEq(vault.totalAssets(), MIN_INITIAL_DEPOSIT, "total assets"); assertEq(vault.totalSupply(), MIN_INITIAL_DEPOSIT, "total supply"); + + assertEq(vault.balanceOf(address(factory)), 1e3, "factory gets 1e3 shares"); + assertEq(vault.balanceOf(address(this)), MIN_INITIAL_DEPOSIT - 1e3, "deployer gets 1e3 less shares"); } function test_CreateVault_Twice() public { @@ -214,4 +219,102 @@ contract VaultFactoryTest is VaultSharedSetup { require(address(vault) != address(vault2), "different deployment address"); } + + /** + * The amount of funds that the attacker can cause the user to lose should + * cost the attacker a significant amount of funds. + */ + function test_InflationAttackCostToGriefShouldBeHigh_DeployerIsNotTheAttacker() public { + address deployer = newAddress("DEPLOYER"); + // deploy using the factory which enforces minimum deposit of 1e9 assets + // and the 1e3 shares burn. + bytes32 salt = keccak256("random salt"); + + setERC20Balance(address(BASE_ASSET), deployer, MIN_INITIAL_DEPOSIT); + + vm.startPrank(deployer); + BASE_ASSET.approve(address(factory), MIN_INITIAL_DEPOSIT); + + Vault vault = factory.createVault( + BASE_ASSET, + feeRecipient, + feePercentage, + name, + symbol, + INITIAL_DELAY, + VAULT_ADMIN, + salt, + marketsArgs, + MIN_INITIAL_DEPOSIT + ); + vm.stopPrank(); + + vm.startPrank(VAULT_ADMIN); + vault.grantRole(vault.OWNER_ROLE(), OWNER); + vm.stopPrank(); + + updateAllocationCaps(vault, type(uint256).max, type(uint256).max, type(uint256).max); + + uint256 donationAmt = 11e18; + uint256 mintAmt = 10; + + // fund attacker + setERC20Balance(address(BASE_ASSET), address(this), donationAmt + mintAmt); + BASE_ASSET.approve(address(vault), type(uint256).max); + + uint256 initialAssetBalance = BASE_ASSET.balanceOf(address(this)); + console2.log("attacker balance before : "); + console2.log(initialAssetBalance); + + vault.mint(mintAmt, address(this)); + uint256 attackerClaimAfterMint = vault.previewRedeem(vault.balanceOf(address(this))); + + console2.log("attackerClaimAfterMint: "); + console2.log(attackerClaimAfterMint); + + console2.log("donationAmt: "); + console2.log(donationAmt); + + // donate to inflate exchange rate by increasing `totalAssets` + IERC20(address(BASE_ASSET)).transfer(address(vault), donationAmt); + + // how much of this donation was captured by the virtual shares on the vault? + uint256 attackerClaimAfterDonation = vault.previewRedeem(vault.balanceOf(address(this))); + + console2.log("attackerClaimAfterDonation: "); + console2.log(attackerClaimAfterDonation); + + uint256 lossFromDonation = attackerClaimAfterMint + donationAmt - attackerClaimAfterDonation; + + console2.log("loss from donation: "); + console2.log(lossFromDonation); + + address alice = address(0xabcd); + setERC20Balance(address(BASE_ASSET), alice, 10e18 + 10); + + vm.startPrank(alice); + IERC20(address(BASE_ASSET)).approve(address(vault), 1e18); + vault.deposit(1e18, alice); + vm.stopPrank(); + + // Alice gained zero shares due to exchange rate inflation + uint256 aliceShares = vault.balanceOf(alice); + console2.log("alice must lose all her shares : "); + console2.log(aliceShares); + + // How much of alice's deposits were captured by the attacker's shares? + uint256 attackerClaimAfterAlice = vault.previewRedeem(vault.balanceOf(address(this))); + uint256 attackerGainFromAlice = attackerClaimAfterAlice - attackerClaimAfterDonation; + console2.log("attackerGainFromAlice: "); + console2.log(attackerGainFromAlice); + + vault.redeem(vault.balanceOf(address(this)) - 3, address(this), address(this)); + uint256 afterAssetBalance = BASE_ASSET.balanceOf(address(this)); + + console2.log("attacker balance after : "); + console2.log(afterAssetBalance); + + assertLe(attackerGainFromAlice, lossFromDonation, "attack must not be profitable"); + assertLe(afterAssetBalance, initialAssetBalance, "attacker must not be profitable"); + } } diff --git a/test/unit/fuzz/vault/Vault.t.sol b/test/unit/fuzz/vault/Vault.t.sol index ef5c6767..6602d6e1 100644 --- a/test/unit/fuzz/vault/Vault.t.sol +++ b/test/unit/fuzz/vault/Vault.t.sol @@ -1,12 +1,13 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.21; +import { Vault } from "./../../../../src/vault/Vault.sol"; +import { VaultFactory } from "./../../../../src/vault/VaultFactory.sol"; import { IIonPool } from "./../../../../src/interfaces/IIonPool.sol"; import { IonPoolExposed } from "../../../helpers/IonPoolSharedSetup.sol"; import { VaultSharedSetup } from "../../../helpers/VaultSharedSetup.sol"; import { Math } from "openzeppelin-contracts/contracts/utils/math/Math.sol"; -import { WadRayMath, RAY, WAD } from "./../../../../src/libraries/math/WadRayMath.sol"; -import { console2 } from "forge-std/console2.sol"; +import { RAY } from "./../../../../src/libraries/math/WadRayMath.sol"; import { IERC20 } from "openzeppelin-contracts/contracts/interfaces/IERC20.sol"; using Math for uint256; @@ -268,5 +269,118 @@ contract VaultInflationAttack is VaultSharedSetup { assertLe(userDepositAmt, attackerLossFromDonation, "loss must be ge to user deposit"); } + // Even though virtual assets and shares makes the attack not 'profitable' + // for the attacker, the attacker may still be able to cause loss of user + // funds for a small loss of their own. For example, the attacker may try to + // cause the user to lose their 1e18 deposit by losing 0.01e18 deposit of + // their own to grief the user, regardless of economic incentives. If the + // vault is deployed through a factory that enforces a minimum deposit and a + // 1e3 shares burn, the attacker should not be able to grief a larger amount + // than they will lose from their own deposits. + function testFuzz_InflationAttackTheAttackerLosesMoreThanItCanGrief(uint256 assets) public { + // Set up factory deployment args with IDLE pool. + uint256[] memory alloCaps = new uint256[](4); + alloCaps[0] = type(uint256).max; + alloCaps[1] = type(uint256).max; + alloCaps[2] = type(uint256).max; + alloCaps[3] = type(uint256).max; + + IIonPool[] memory markets = new IIonPool[](4); + markets[0] = IDLE; + markets[1] = weEthIonPool; + markets[2] = rsEthIonPool; + markets[3] = rswEthIonPool; + + marketsArgs.marketsToAdd = markets; + marketsArgs.allocationCaps = alloCaps; + marketsArgs.newSupplyQueue = markets; + marketsArgs.newWithdrawQueue = markets; + + address deployer = newAddress("DEPLOYER"); + + // deploy using the factory which enforces minimum deposit of 1e9 assets + // and the 1e3 shares burn. + bytes32 salt = keccak256("random salt"); + + setERC20Balance(address(BASE_ASSET), deployer, MIN_INITIAL_DEPOSIT); + + VaultFactory factory = new VaultFactory(); + + vm.startPrank(deployer); + BASE_ASSET.approve(address(factory), MIN_INITIAL_DEPOSIT); + + Vault vault = factory.createVault( + BASE_ASSET, + FEE_RECIPIENT, + ZERO_FEES, + "Ion Vault Token", + "IVT", + INITIAL_DELAY, + VAULT_ADMIN, + salt, + marketsArgs, + MIN_INITIAL_DEPOSIT + ); + vm.stopPrank(); + + vm.startPrank(VAULT_ADMIN); + vault.grantRole(vault.OWNER_ROLE(), OWNER); + vm.stopPrank(); + + // 1. The vault has not been used. + // - Initial minimum deposit amt of 1e9 deposited. + // - 1e3 shares have been locked in factory. + assertEq(vault.totalSupply(), MIN_INITIAL_DEPOSIT, "initial total supply"); + assertEq(vault.totalAssets(), MIN_INITIAL_DEPOSIT, "initial total assets"); + assertEq(vault.balanceOf(address(factory)), 1e3, "initial factory shares"); + + // 2. The attacker makes a first deposit. + uint256 firstDepositAmt = bound(assets, 1, type(uint128).max); + setERC20Balance(address(BASE_ASSET), ATTACKER, firstDepositAmt); + + vm.startPrank(ATTACKER); + BASE_ASSET.approve(address(vault), type(uint256).max); + vault.mint(firstDepositAmt, ATTACKER); + vm.stopPrank(); + + uint256 attackerClaimAfterMint = vault.previewRedeem(vault.balanceOf(ATTACKER)); + + assertEq(BASE_ASSET.balanceOf(ATTACKER), 0, "mint amount equals transfer amount"); + + // 3. The attacker donates. + // - In this case, transfers to vault to increase IDLE deposits. + // - Check that the attacker loses a portion of the donated funds. + uint256 donationAmt = bound(assets, firstDepositAmt, type(uint128).max); + setERC20Balance(address(BASE_ASSET), ATTACKER, donationAmt); + + vm.prank(ATTACKER); + IERC20(address(BASE_ASSET)).transfer(address(vault), donationAmt); + + uint256 attackerClaimAfterDonation = vault.previewRedeem(vault.balanceOf(ATTACKER)); + uint256 attackerLossFromDonation = donationAmt - (attackerClaimAfterDonation - attackerClaimAfterMint); + + // 4. A user makes a deposit where the shares truncate to zero. + // - sharesToMint = depositAmt * (newTotalSupply + 1) / (newTotalAssets + 1) + // - The sharesToMint must be less than 1 to round down to zero + // - depositAmt * (newTotalSupply + 1) / (newTotalAssets + 1) < 1 + // - depositAmt < 1 * (newTotalAssets + 1) / (newTotalSupply + 1) + uint256 maxDepositAmt = (vault.totalAssets() + 1) / (vault.totalSupply() + 1); + uint256 userDepositAmt = bound(assets, 1, maxDepositAmt); + + vm.startPrank(USER); + setERC20Balance(address(BASE_ASSET), USER, userDepositAmt); + IERC20(address(BASE_ASSET)).approve(address(vault), userDepositAmt); + vault.deposit(userDepositAmt, USER); + vm.stopPrank(); + + assertEq(vault.balanceOf(USER), 0, "user minted shares must be zero"); + + uint256 attackerClaimAfterUser = vault.previewRedeem(vault.balanceOf(ATTACKER)); + uint256 attackerGainFromUser = attackerClaimAfterUser - attackerClaimAfterDonation; + + uint256 attackerNetLoss = firstDepositAmt + donationAmt - attackerClaimAfterUser; + assertLe(userDepositAmt, attackerNetLoss, "attacker net loss greater than user deposit amt"); + } + function testFuzz_InflationAttackSmallerDegree() public { } } From 207464290a6178b67083c0c88900c93e0e3d993a Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Tue, 14 May 2024 00:53:49 -0400 Subject: [PATCH 19/20] chore: fix natspec --- src/vault/VaultFactory.sol | 11 +++-- test/fork/concrete/vault/VaultFactory.t.sol | 53 ++++++++++++++++----- test/helpers/VaultSharedSetup.sol | 2 +- 3 files changed, 50 insertions(+), 16 deletions(-) diff --git a/src/vault/VaultFactory.sol b/src/vault/VaultFactory.sol index 744da428..b2915058 100644 --- a/src/vault/VaultFactory.sol +++ b/src/vault/VaultFactory.sol @@ -28,10 +28,13 @@ contract VaultFactory { // --- External --- /** - * @notice Deploys a new Ion Lending Vault. Transfers 1 gwei of base asset - * from the caller to initiate an initial deposit to the vault. - * @dev The 1e9 initial deposit amount was chosen to defend against - * inflation attacks. + * @notice Deploys a new Ion Lending Vault. Transfers the `initialDeposit` + * amount of the base asset from the caller initiate the first deposit to + * the vault. The minimum `initialDeposit` is 1e3. If less, this call would + * underflow as it will always burn 1e3 shares of the total shares minted to + * defend against inflation attacks. + * @dev The 1e3 initial deposit amount was chosen to defend against + * inflation attacks, referencing the UniV2 LP token implementation. * @param baseAsset The asset that is being lent out to IonPools. * @param feeRecipient Address that receives the accrued manager fees. * @param feePercentage Fee percentage to be set. diff --git a/test/fork/concrete/vault/VaultFactory.t.sol b/test/fork/concrete/vault/VaultFactory.t.sol index 9e59f349..b7201181 100644 --- a/test/fork/concrete/vault/VaultFactory.t.sol +++ b/test/fork/concrete/vault/VaultFactory.t.sol @@ -225,6 +225,23 @@ contract VaultFactoryTest is VaultSharedSetup { * cost the attacker a significant amount of funds. */ function test_InflationAttackCostToGriefShouldBeHigh_DeployerIsNotTheAttacker() public { + uint256[] memory alloCaps = new uint256[](4); + alloCaps[0] = type(uint256).max; + alloCaps[1] = type(uint256).max; + alloCaps[2] = type(uint256).max; + alloCaps[3] = type(uint256).max; + + IIonPool[] memory markets = new IIonPool[](4); + markets[0] = IDLE; + markets[1] = weEthIonPool; + markets[2] = rsEthIonPool; + markets[3] = rswEthIonPool; + + marketsArgs.marketsToAdd = markets; + marketsArgs.allocationCaps = alloCaps; + marketsArgs.newSupplyQueue = markets; + marketsArgs.newWithdrawQueue = markets; + address deployer = newAddress("DEPLOYER"); // deploy using the factory which enforces minimum deposit of 1e9 assets // and the 1e3 shares burn. @@ -255,7 +272,7 @@ contract VaultFactoryTest is VaultSharedSetup { updateAllocationCaps(vault, type(uint256).max, type(uint256).max, type(uint256).max); - uint256 donationAmt = 11e18; + uint256 donationAmt = 10e18; uint256 mintAmt = 10; // fund attacker @@ -263,31 +280,34 @@ contract VaultFactoryTest is VaultSharedSetup { BASE_ASSET.approve(address(vault), type(uint256).max); uint256 initialAssetBalance = BASE_ASSET.balanceOf(address(this)); - console2.log("attacker balance before : "); - console2.log(initialAssetBalance); + console2.log("attacker balance before :"); + console2.log("%e", initialAssetBalance); vault.mint(mintAmt, address(this)); uint256 attackerClaimAfterMint = vault.previewRedeem(vault.balanceOf(address(this))); console2.log("attackerClaimAfterMint: "); - console2.log(attackerClaimAfterMint); + console2.log("%e", attackerClaimAfterMint); console2.log("donationAmt: "); - console2.log(donationAmt); + console2.log("%e", donationAmt); // donate to inflate exchange rate by increasing `totalAssets` IERC20(address(BASE_ASSET)).transfer(address(vault), donationAmt); + assertEq(donationAmt + mintAmt + 1e3, vault.totalAssets(), "total assets"); + assertEq(mintAmt + 1e3, vault.totalSupply(), "minted shares"); + // how much of this donation was captured by the virtual shares on the vault? uint256 attackerClaimAfterDonation = vault.previewRedeem(vault.balanceOf(address(this))); console2.log("attackerClaimAfterDonation: "); - console2.log(attackerClaimAfterDonation); + console2.log("%e", attackerClaimAfterDonation); uint256 lossFromDonation = attackerClaimAfterMint + donationAmt - attackerClaimAfterDonation; console2.log("loss from donation: "); - console2.log(lossFromDonation); + console2.log("%e", lossFromDonation); address alice = address(0xabcd); setERC20Balance(address(BASE_ASSET), alice, 10e18 + 10); @@ -299,22 +319,33 @@ contract VaultFactoryTest is VaultSharedSetup { // Alice gained zero shares due to exchange rate inflation uint256 aliceShares = vault.balanceOf(alice); - console2.log("alice must lose all her shares : "); - console2.log(aliceShares); + console2.log("alice resulting shares : "); + console2.log("%e", aliceShares); + + uint256 aliceClaim = vault.maxWithdraw(alice); + console2.log("alice resulting claim: "); + console2.log("%e", aliceClaim); + + console2.log("alice resulting assets lost: "); + console2.log("%e", 1e18 - aliceClaim); // How much of alice's deposits were captured by the attacker's shares? uint256 attackerClaimAfterAlice = vault.previewRedeem(vault.balanceOf(address(this))); uint256 attackerGainFromAlice = attackerClaimAfterAlice - attackerClaimAfterDonation; console2.log("attackerGainFromAlice: "); - console2.log(attackerGainFromAlice); + console2.log("%e", attackerGainFromAlice); vault.redeem(vault.balanceOf(address(this)) - 3, address(this), address(this)); uint256 afterAssetBalance = BASE_ASSET.balanceOf(address(this)); console2.log("attacker balance after : "); - console2.log(afterAssetBalance); + console2.log("%e", afterAssetBalance); + + console2.log("attacker loss in balance"); + console2.log("%e", initialAssetBalance - afterAssetBalance); assertLe(attackerGainFromAlice, lossFromDonation, "attack must not be profitable"); assertLe(afterAssetBalance, initialAssetBalance, "attacker must not be profitable"); + assertLe(1e18, initialAssetBalance - afterAssetBalance, "attacker loss greater than amount griefed"); } } diff --git a/test/helpers/VaultSharedSetup.sol b/test/helpers/VaultSharedSetup.sol index 84650c63..80cdb421 100644 --- a/test/helpers/VaultSharedSetup.sol +++ b/test/helpers/VaultSharedSetup.sol @@ -44,7 +44,7 @@ contract VaultSharedSetup is IonPoolSharedSetup { address constant FEE_RECIPIENT = address(uint160(uint256(keccak256("FEE_RECIPIENT")))); uint256 constant ZERO_FEES = 0; - uint256 constant MIN_INITIAL_DEPOSIT = 1e9; + uint256 constant MIN_INITIAL_DEPOSIT = 1e3; bytes32 constant SALT = keccak256("SALT"); From 1c826f4fbeb791f495e661b591a066bd4dd0f4db Mon Sep 17 00:00:00 2001 From: Jun Kim <64379343+junkim012@users.noreply.github.com> Date: Thu, 16 May 2024 11:57:14 -0400 Subject: [PATCH 20/20] chore: solhint disable next line for empty code blocks and import for IERC4626 inheritdoc --- src/vault/Vault.sol | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/vault/Vault.sol b/src/vault/Vault.sol index a07bf3b2..b4854dd8 100644 --- a/src/vault/Vault.sol +++ b/src/vault/Vault.sol @@ -6,7 +6,6 @@ import { IIonPool } from "./../interfaces/IIonPool.sol"; import { RAY } from "./../libraries/math/WadRayMath.sol"; import { IERC20Metadata } from "@openzeppelin/contracts/interfaces/IERC20Metadata.sol"; -import { IERC4626 } from "@openzeppelin/contracts/interfaces/IERC4626.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { ERC4626 } from "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; @@ -16,6 +15,10 @@ import { Multicall } from "@openzeppelin/contracts/utils/Multicall.sol"; import { ReentrancyGuard } from "openzeppelin-contracts/contracts/utils/ReentrancyGuard.sol"; import { AccessControlDefaultAdminRules } from "@openzeppelin/contracts/access/extensions/AccessControlDefaultAdminRules.sol"; + +// solhint-disable-next-line no-unused-import +import { IERC4626 } from "@openzeppelin/contracts/interfaces/IERC4626.sol"; + /** * @title Ion Lending Vault * @author Molecular Labs @@ -27,7 +30,6 @@ import { AccessControlDefaultAdminRules } from * * @custom:security-contact security@molecularlabs.io */ - contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, ReentrancyGuard { using EnumerableSet for EnumerableSet.AddressSet; using Math for uint256; @@ -470,6 +472,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy if (pool != IDLE) { try pool.supply(address(this), toSupply, new bytes32[](0)) { assets -= toSupply; + // solhint-disable-next-line no-empty-blocks } catch { } } else { assets -= toSupply; @@ -512,6 +515,7 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy if (pool != IDLE) { try pool.withdraw(address(this), toWithdraw) { assets -= toWithdraw; + // solhint-disable-next-line no-empty-blocks } catch { } } else { assets -= toWithdraw;