fix: querying unaccrued interest when paused, caching IDLE balance in…

… reallocate, accrue fee when feePercentage is updated

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);
 }

src/token/RewardToken.sol

@@ -450,7 +450,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 +461,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 getBalanceOfUnaccrued(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

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 [email protected]
  */
+
 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);
     }

test/unit/concrete/vault/Vault.t.sol

@@ -1495,6 +1495,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();

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 { }
+}