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..b2915058 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,13 @@ contract VaultFactory { // --- External --- /** - * @notice Deploys a new Ion Lending Vault. + * @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. @@ -34,6 +43,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 +54,25 @@ 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); + 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 1715b1a3..b7201181 100644 --- a/test/fork/concrete/vault/VaultFactory.t.sol +++ b/test/fork/concrete/vault/VaultFactory.t.sol @@ -6,6 +6,9 @@ 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"; + +import { console2 } from "forge-std/console2.sol"; contract VaultFactoryTest is VaultSharedSetup { VaultFactory factory; @@ -16,30 +19,119 @@ 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"); + + 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 { 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,26 +144,208 @@ 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"); } + + /** + * 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 { + 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); + + 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 = 10e18; + 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("%e", initialAssetBalance); + + vault.mint(mintAmt, address(this)); + uint256 attackerClaimAfterMint = vault.previewRedeem(vault.balanceOf(address(this))); + + console2.log("attackerClaimAfterMint: "); + console2.log("%e", attackerClaimAfterMint); + + 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("%e", attackerClaimAfterDonation); + + uint256 lossFromDonation = attackerClaimAfterMint + donationAmt - attackerClaimAfterDonation; + + console2.log("loss from donation: "); + console2.log("%e", 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 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("%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("%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 39618c79..80cdb421 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 = 1e3; + + 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..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; @@ -124,8 +125,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); @@ -267,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 { } }