Skip to content

Commit

Permalink
Merge pull request #90 from Ion-Protocol/jun/PAG-M-08
Browse files Browse the repository at this point in the history
M-08 Fix [PAG]
  • Loading branch information
junkim012 authored May 16, 2024
2 parents 226ebd8 + 9c9a020 commit c85a053
Show file tree
Hide file tree
Showing 5 changed files with 152 additions and 16 deletions.
10 changes: 5 additions & 5 deletions src/IonPool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -493,14 +493,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
Expand Down
11 changes: 10 additions & 1 deletion src/vault/Vault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -194,6 +197,8 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy
}
}

if (supportedMarkets.length() > MAX_SUPPORTED_MARKETS) revert MaxSupportedMarketsReached();

_updateSupplyQueue(newSupplyQueue);
_updateWithdrawQueue(newWithdrawQueue);
}
Expand Down Expand Up @@ -947,9 +952,11 @@ 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.balanceOf(address(this));
uint256 availableLiquidity = uint256(pool.extsload(ION_POOL_LIQUIDITY_SLOT));

uint256 availableLiquidity = uint256(pool.extsload(ION_POOL_LIQUIDITY_SLOT));
return Math.min(currentSupplied, availableLiquidity);
}

Expand All @@ -960,6 +967,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.balanceOf(address(this)));
uint256 supplyCapDiff = _zeroFloorSub(uint256(pool.extsload(ION_POOL_SUPPLY_CAP_SLOT)), pool.totalSupply());

Expand Down
46 changes: 46 additions & 0 deletions test/unit/concrete/IonPool.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
82 changes: 82 additions & 0 deletions test/unit/concrete/vault/Vault.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,30 @@ 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, emptyMarketsArgs
);

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;
Expand Down Expand Up @@ -1694,6 +1718,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
Expand Down
19 changes: 9 additions & 10 deletions test/unit/fuzz/vault/Vault.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ contract Vault_Fuzz is VaultSharedSetup {

uint256 initialDeposit = bound(assets, 1e18, supplyCap - 10e18);
supply(address(this), weEthIonPool, initialDeposit);
uint256 initialTotalNormalized = weEthIonPool.totalSupplyUnaccrued();
uint256 initialTotalNormalized = weEthIonPool.normalizedTotalSupply();

uint256 supplyCapDiff = _zeroFloorSub(supplyCap, weEthIonPool.getTotalUnderlyingClaims());
uint256 supplyCapDiff = _zeroFloorSub(supplyCap, weEthIonPool.totalSupply());

// `IonPool.supply` math
uint256 amountScaled = supplyCapDiff.rayDivDown(supplyFactor);
Expand All @@ -83,21 +83,19 @@ contract Vault_Fuzz is VaultSharedSetup {

supply(address(this), weEthIonPool, supplyCapDiff);

assertEq(
resultingTotalClaim, weEthIonPool.getTotalUnderlyingClaims(), "resulting should be the same as calculated"
);
assertEq(resultingTotalClaim, weEthIonPool.totalSupply(), "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");
assertLe(weEthIonPool.totalSupply(), 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);
supplyFactor = bound(supplyFactor, 1e27, 9e27);
IonPoolExposed(address(weEthIonPool)).setSupplyFactor(supplyFactor);

uint256 allocationCap = bound(assets, 100e18, type(uint128).max);
Expand All @@ -108,9 +106,10 @@ contract Vault_Fuzz is VaultSharedSetup {
setERC20Balance(address(BASE_ASSET), address(this), depositAmt);
vault.deposit(depositAmt, address(this));

uint256 initialTotalNormalized = weEthIonPool.totalSupplyUnaccrued();
// uint256 initialTotalNormalized = weEthIonPool.totalSupplyUnaccrued();
uint256 initialTotalNormalized = weEthIonPool.normalizedTotalSupply();

uint256 allocationCapDiff = _zeroFloorSub(allocationCap, weEthIonPool.getUnderlyingClaimOf(address(vault)));
uint256 allocationCapDiff = _zeroFloorSub(allocationCap, weEthIonPool.balanceOf(address(vault)));

uint256 amountScaled = allocationCapDiff.rayDivDown(supplyFactor);
uint256 resultingTotalNormalized = initialTotalNormalized + amountScaled;
Expand All @@ -123,7 +122,7 @@ contract Vault_Fuzz is VaultSharedSetup {
setERC20Balance(address(BASE_ASSET), address(this), allocationCapDiff + 123e18);
vault.deposit(allocationCapDiff + 123e18, address(this));

uint256 actualTotalClaim = weEthIonPool.getUnderlyingClaimOf(address(vault));
uint256 actualTotalClaim = weEthIonPool.balanceOf(address(vault));
assertEq(resultingTotalClaim, actualTotalClaim, "expected and actual must be equal");

assertLe(resultingTotalClaim, allocationCap, "expected claim le to allocation cap");
Expand Down

0 comments on commit c85a053

Please sign in to comment.