From 32b1b6127726d27935119868cf347a8cbb74cc73 Mon Sep 17 00:00:00 2001 From: Pierrick Turelier Date: Fri, 11 Aug 2023 19:19:36 -0500 Subject: [PATCH 1/2] fix(exchangeRate): add 1:1 exchange rate --- lcov.info | 547 +++++++++--------- src/Vault.sol | 137 +++-- test/contracts/mock/YieldVault.sol | 28 +- test/unit/Vault/Deposit.feature | 19 +- test/unit/Vault/Deposit.t.sol | 80 ++- .../unit/Vault/Undercollateralization.feature | 16 + test/unit/Vault/Undercollateralization.t.sol | 116 +++- test/unit/Vault/Withdraw.feature | 22 + test/unit/Vault/Withdraw.t.sol | 88 ++- 9 files changed, 629 insertions(+), 424 deletions(-) diff --git a/lcov.info b/lcov.info index 26cffb7..615a3c9 100644 --- a/lcov.info +++ b/lcov.info @@ -1,96 +1,95 @@ TN: SF:src/Vault.sol -FN:1011,Vault._withdraw -FN:1049,Vault._claimPrize -FN:1100,Vault._permit -FN:1116,Vault._updateExchangeRate -FN:1129,Vault._mint -FN:1147,Vault._burn -FN:1162,Vault._transfer -FN:1177,Vault._currentExchangeRate -FN:1206,Vault._isVaultCollateralized -FN:1211,Vault._requireVaultCollateralized -FN:1221,Vault._setClaimer -FN:1230,Vault._setYieldFeePercentage -FN:1241,Vault._setYieldFeeRecipient -FN:318,Vault.availableYieldBalance -FN:329,Vault.availableYieldFeeBalance -FN:340,Vault.balanceOf -FN:347,Vault.decimals -FN:352,Vault.totalAssets -FN:357,Vault.totalSupply -FN:366,Vault.exchangeRate -FN:374,Vault.isVaultCollateralized -FN:382,Vault.maxDeposit -FN:395,Vault.maxMint -FN:411,Vault.mintYieldFee -FN:429,Vault.deposit -FN:449,Vault.depositWithPermit -FN:462,Vault.mint -FN:480,Vault.mintWithPermit -FN:502,Vault.sponsor -FN:516,Vault.sponsorWithPermit -FN:531,Vault.withdraw -FN:546,Vault.redeem -FN:562,Vault.liquidatableBalanceOf -FN:572,Vault.liquidate -FN:618,Vault.targetOf -FN:634,Vault.claimPrizes -FN:668,Vault.setClaimer -FN:680,Vault.setHooks -FN:692,Vault.setLiquidationPair -FN:718,Vault.setYieldFeePercentage -FN:731,Vault.setYieldFeeRecipient -FN:746,Vault.yieldFeeRecipient -FN:755,Vault.yieldFeePercentage -FN:764,Vault.yieldFeeShares -FN:772,Vault.twabController -FN:780,Vault.yieldVault -FN:788,Vault.liquidationPair -FN:796,Vault.prizePool -FN:804,Vault.claimer -FN:813,Vault.getHooks -FN:825,Vault._totalAssets -FN:833,Vault._totalSupply -FN:844,Vault._liquidatableBalanceOf -FN:859,Vault._availableYieldFeeBalance -FN:867,Vault._increaseYieldFeeBalance -FN:879,Vault._convertToShares -FN:897,Vault._convertToAssets -FN:940,Vault._deposit -FN:986,Vault._sponsor -FNDA:3338,Vault._withdraw +FN:1025,Vault._sponsor +FN:1050,Vault._withdraw +FN:1088,Vault._claimPrize +FN:1139,Vault._permit +FN:1162,Vault._mint +FN:1180,Vault._burn +FN:1195,Vault._transfer +FN:1214,Vault._exchangeRate +FN:1241,Vault._isVaultCollateralized +FN:1246,Vault._requireVaultCollateralized +FN:1256,Vault._setClaimer +FN:1265,Vault._setYieldFeePercentage +FN:1276,Vault._setYieldFeeRecipient +FN:369,Vault.availableYieldBalance +FN:383,Vault.availableYieldFeeBalance +FN:394,Vault.balanceOf +FN:401,Vault.decimals +FN:406,Vault.totalAssets +FN:411,Vault.totalSupply +FN:420,Vault.exchangeRate +FN:428,Vault.isVaultCollateralized +FN:436,Vault.maxDeposit +FN:450,Vault.maxMint +FN:466,Vault.mintYieldFee +FN:485,Vault.deposit +FN:507,Vault.depositWithPermit +FN:520,Vault.mint +FN:536,Vault.sponsor +FN:550,Vault.sponsorWithPermit +FN:567,Vault.sweep +FN:581,Vault.withdraw +FN:596,Vault.redeem +FN:612,Vault.liquidatableBalanceOf +FN:622,Vault.liquidate +FN:662,Vault.targetOf +FN:678,Vault.claimPrizes +FN:712,Vault.setClaimer +FN:724,Vault.setHooks +FN:736,Vault.setLiquidationPair +FN:762,Vault.setYieldFeePercentage +FN:775,Vault.setYieldFeeRecipient +FN:790,Vault.yieldFeeRecipient +FN:799,Vault.yieldFeePercentage +FN:808,Vault.yieldFeeShares +FN:816,Vault.twabController +FN:824,Vault.yieldVault +FN:832,Vault.liquidationPair +FN:840,Vault.prizePool +FN:848,Vault.claimer +FN:857,Vault.getHooks +FN:869,Vault._totalAssets +FN:877,Vault._totalSupply +FN:888,Vault._liquidatableBalanceOf +FN:903,Vault._availableYieldFeeBalance +FN:911,Vault._increaseYieldFeeBalance +FN:923,Vault._convertToShares +FN:939,Vault._convertToAssets +FN:968,Vault._deposit +FNDA:16,Vault._sponsor +FNDA:3377,Vault._withdraw FNDA:3,Vault._claimPrize -FNDA:6,Vault._permit -FNDA:36222,Vault._updateExchangeRate -FNDA:33119,Vault._mint -FNDA:3107,Vault._burn +FNDA:4,Vault._permit +FNDA:33121,Vault._mint +FNDA:3108,Vault._burn FNDA:258,Vault._transfer -FNDA:175595,Vault._currentExchangeRate -FNDA:65242,Vault._isVaultCollateralized -FNDA:279,Vault._requireVaultCollateralized +FNDA:36878,Vault._exchangeRate +FNDA:98111,Vault._isVaultCollateralized +FNDA:33138,Vault._requireVaultCollateralized FNDA:2,Vault._setClaimer FNDA:13,Vault._setYieldFeePercentage FNDA:5,Vault._setYieldFeeRecipient -FNDA:9,Vault.availableYieldBalance +FNDA:10,Vault.availableYieldBalance FNDA:9,Vault.availableYieldFeeBalance -FNDA:3669,Vault.balanceOf +FNDA:3671,Vault.balanceOf FNDA:1,Vault.decimals FNDA:1024,Vault.totalAssets -FNDA:2341,Vault.totalSupply -FNDA:2,Vault.exchangeRate -FNDA:12,Vault.isVaultCollateralized +FNDA:2342,Vault.totalSupply +FNDA:8,Vault.exchangeRate +FNDA:16,Vault.isVaultCollateralized FNDA:259,Vault.maxDeposit FNDA:256,Vault.maxMint FNDA:5,Vault.mintYieldFee -FNDA:31292,Vault.deposit +FNDA:31298,Vault.deposit FNDA:2,Vault.depositWithPermit -FNDA:1540,Vault.mint -FNDA:2,Vault.mintWithPermit +FNDA:1543,Vault.mint FNDA:14,Vault.sponsor FNDA:2,Vault.sponsorWithPermit -FNDA:1824,Vault.withdraw -FNDA:1797,Vault.redeem +FNDA:2,Vault.sweep +FNDA:1828,Vault.withdraw +FNDA:1798,Vault.redeem FNDA:1318,Vault.liquidatableBalanceOf FNDA:274,Vault.liquidate FNDA:1,Vault.targetOf @@ -109,205 +108,207 @@ FNDA:2,Vault.liquidationPair FNDA:2,Vault.prizePool FNDA:3,Vault.claimer FNDA:1,Vault.getHooks -FNDA:2629,Vault._totalAssets -FNDA:179545,Vault._totalSupply +FNDA:2630,Vault._totalAssets +FNDA:183314,Vault._totalSupply FNDA:1587,Vault._liquidatableBalanceOf FNDA:1589,Vault._availableYieldFeeBalance FNDA:9,Vault._increaseYieldFeeBalance -FNDA:34015,Vault._convertToShares -FNDA:255823,Vault._convertToAssets -FNDA:32849,Vault._deposit -FNDA:16,Vault._sponsor -FNF:59 -FNH:59 -DA:319,1605 -DA:320,1605 -DA:322,1605 -DA:330,9 -DA:332,9 -DA:333,2 -DA:336,7 -DA:343,72897 -DA:348,1 -DA:353,1024 -DA:358,2341 -DA:367,2 -DA:375,12 -DA:383,31572 -DA:385,31490 -DA:386,31490 -DA:388,31490 -DA:396,33379 -DA:398,33272 -DA:399,33272 -DA:401,33272 -DA:412,5 -DA:414,4 -DA:415,4 -DA:417,4 -DA:418,3 -DA:420,2 -DA:421,2 -DA:423,1 -DA:430,31310 -DA:431,3 -DA:433,31307 -DA:434,31307 -DA:436,31307 -DA:457,2 -DA:458,2 -DA:463,1540 -DA:465,1540 -DA:467,1538 -DA:488,2 -DA:490,2 -DA:491,2 -DA:493,2 -DA:503,14 -DA:524,2 -DA:525,2 -DA:536,1824 -DA:537,140 -DA:539,1684 -DA:540,1684 -DA:542,1567 -DA:551,1797 -DA:553,1654 -DA:554,1654 -DA:556,1540 -DA:563,1318 -DA:579,274 -DA:581,273 -DA:582,1 -DA:584,272 -DA:585,1 -DA:587,271 -DA:588,1 -DA:590,270 -DA:592,269 -DA:593,269 -DA:595,269 -DA:596,1 -DA:598,268 -DA:600,268 -DA:601,9 -DA:606,268 -DA:608,268 -DA:609,0 -DA:612,268 -DA:614,267 -DA:619,1 -DA:641,5 -DA:643,3 -DA:645,3 -DA:646,3 -DA:647,3 -DA:648,3 -DA:658,3 -DA:669,2 -DA:670,2 -DA:672,2 -DA:673,2 -DA:681,3 -DA:683,3 -DA:695,280 -DA:697,279 -DA:698,279 -DA:700,279 -DA:701,1 -DA:704,279 -DA:706,279 -DA:708,279 -DA:709,279 -DA:719,13 -DA:720,13 -DA:722,12 -DA:723,12 -DA:732,5 -DA:733,5 -DA:735,5 -DA:736,5 -DA:747,2 -DA:756,2 -DA:765,7 -DA:773,2 -DA:781,2 -DA:789,2 -DA:797,2 -DA:805,3 -DA:814,1 -DA:826,2629 -DA:834,179545 -DA:845,1587 -DA:847,1587 -DA:850,1587 -DA:860,1589 -DA:868,9 -DA:883,34015 -DA:885,34015 -DA:886,34015 -DA:901,40114 -DA:916,215709 -DA:917,215709 -DA:946,32849 -DA:947,32849 -DA:957,32849 -DA:958,30011 -DA:961,30011 -DA:962,1 -DA:966,30011 -DA:974,32849 -DA:975,32849 -DA:977,32847 -DA:987,16 -DA:990,16 -DA:992,15 -DA:995,16 -DA:997,16 -DA:1018,3338 -DA:1019,1257 -DA:1028,3107 -DA:1030,3107 -DA:1031,3107 -DA:1033,3107 -DA:1035,3107 -DA:1056,3 -DA:1057,3 -DA:1059,3 -DA:1060,2 -DA:1062,1 -DA:1065,3 -DA:1074,3 -DA:1075,1 -DA:1084,3 -DA:1110,6 -DA:1117,36222 -DA:1118,36222 -DA:1130,33119 -DA:1132,33115 -DA:1133,33115 -DA:1135,33115 -DA:1148,3107 -DA:1150,3107 -DA:1163,258 -DA:1165,258 -DA:1178,175595 -DA:1179,175595 -DA:1185,175595 -DA:1190,175595 -DA:1191,51842 -DA:1194,175595 -DA:1195,142610 -DA:1198,32985 -DA:1207,65242 -DA:1212,279 -DA:1222,2 -DA:1231,13 -DA:1232,1 -DA:1234,12 -DA:1242,5 -LF:186 -LH:185 +FNDA:34029,Vault._convertToShares +FNDA:40186,Vault._convertToAssets +FNDA:32853,Vault._deposit +FNF:58 +FNH:58 +DA:370,1606 +DA:371,1606 +DA:376,1606 +DA:384,9 +DA:386,9 +DA:387,2 +DA:390,7 +DA:397,72989 +DA:402,1 +DA:407,1024 +DA:412,2342 +DA:421,8 +DA:429,16 +DA:437,31576 +DA:439,31532 +DA:440,31532 +DA:441,31532 +DA:443,31532 +DA:451,33381 +DA:453,33345 +DA:454,33345 +DA:456,33345 +DA:467,5 +DA:469,4 +DA:470,4 +DA:471,4 +DA:473,4 +DA:474,3 +DA:476,2 +DA:477,2 +DA:479,1 +DA:486,31316 +DA:488,31314 +DA:489,3 +DA:491,31311 +DA:492,31311 +DA:494,31310 +DA:515,2 +DA:516,2 +DA:521,1543 +DA:523,1542 +DA:525,1542 +DA:527,1539 +DA:537,14 +DA:558,2 +DA:559,2 +DA:568,2 +DA:569,2 +DA:571,1 +DA:573,1 +DA:575,1 +DA:586,1828 +DA:587,134 +DA:589,1694 +DA:590,1694 +DA:592,1568 +DA:601,1798 +DA:603,1683 +DA:604,1683 +DA:606,1540 +DA:613,1318 +DA:629,274 +DA:631,273 +DA:632,1 +DA:634,272 +DA:635,1 +DA:637,271 +DA:638,1 +DA:640,270 +DA:642,269 +DA:643,269 +DA:645,269 +DA:646,1 +DA:648,268 +DA:650,268 +DA:651,9 +DA:656,268 +DA:658,267 +DA:663,1 +DA:685,5 +DA:687,3 +DA:689,3 +DA:690,3 +DA:691,3 +DA:692,3 +DA:702,3 +DA:713,2 +DA:714,2 +DA:716,2 +DA:717,2 +DA:725,3 +DA:727,3 +DA:739,280 +DA:741,279 +DA:742,279 +DA:744,279 +DA:745,1 +DA:748,279 +DA:750,279 +DA:752,279 +DA:753,279 +DA:763,13 +DA:764,13 +DA:766,12 +DA:767,12 +DA:776,5 +DA:777,5 +DA:779,5 +DA:780,5 +DA:791,2 +DA:800,2 +DA:809,7 +DA:817,2 +DA:825,2 +DA:833,2 +DA:841,2 +DA:849,3 +DA:858,1 +DA:870,2630 +DA:878,183314 +DA:889,1587 +DA:891,1587 +DA:894,1587 +DA:904,1589 +DA:912,9 +DA:927,34029 +DA:928,34029 +DA:943,40186 +DA:944,40186 +DA:974,32853 +DA:976,32852 +DA:977,32852 +DA:987,32852 +DA:988,32852 +DA:991,32852 +DA:992,1 +DA:996,32852 +DA:1004,32852 +DA:1006,32852 +DA:1008,32852 +DA:1009,32852 +DA:1011,32852 +DA:1012,1 +DA:1014,32851 +DA:1016,32849 +DA:1026,16 +DA:1029,16 +DA:1031,15 +DA:1034,16 +DA:1036,16 +DA:1057,3377 +DA:1059,3369 +DA:1060,1287 +DA:1069,3108 +DA:1071,3108 +DA:1072,3108 +DA:1074,3108 +DA:1095,3 +DA:1096,3 +DA:1098,3 +DA:1099,2 +DA:1101,1 +DA:1104,3 +DA:1113,3 +DA:1114,1 +DA:1123,3 +DA:1149,4 +DA:1163,33121 +DA:1164,4 +DA:1166,33117 +DA:1168,33117 +DA:1181,3108 +DA:1183,3108 +DA:1196,258 +DA:1198,258 +DA:1215,36878 +DA:1216,36878 +DA:1220,36878 +DA:1221,36085 +DA:1226,793 +DA:1227,788 +DA:1232,5 +DA:1242,98111 +DA:1247,33138 +DA:1257,2 +DA:1266,13 +DA:1267,1 +DA:1269,12 +DA:1277,5 +LF:189 +LH:189 end_of_record TN: SF:src/VaultFactory.sol diff --git a/src/Vault.sol b/src/Vault.sol index dfdff7b..ac132cd 100644 --- a/src/Vault.sol +++ b/src/Vault.sol @@ -68,6 +68,9 @@ error MintMoreThanMax(address receiver, uint256 shares, uint256 max); /// @notice Emitted when `_deposit` is called but no shares are minted back to the receiver. error MintZeroShares(); +/// @notice Emitted when `_withdraw` is called but no assets are being withdrawn. +error WithdrawZeroAssets(); + /// @notice Emitted when `sweep` is called but no underlying assets are currently held by the Vault. error SweepZeroAssets(); @@ -102,9 +105,19 @@ error LiquidationAmountOutZero(); */ error LiquidationAmountOutGTYield(uint256 amountOut, uint256 availableYield); -/// @notice Emitted when the vault is under-collateralized. +/// @notice Emitted when the Vault is under-collateralized. error VaultUnderCollateralized(); +/** + * @notice Emitted when after a deposit the amount of withdrawable assets from the YieldVault is lower than the expected amount. + * @param withdrawableAssets The actual amount of assets withdrawable from the YieldVault + * @param expectedWithdrawableAssets The expected amount of assets withdrawable from the YieldVault + */ +error YVWithdrawableAssetsLTExpected( + uint256 withdrawableAssets, + uint256 expectedWithdrawableAssets +); + /** * @notice Emitted when the target token is not supported for a given token address. * @param token The unsupported token address @@ -243,13 +256,6 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { */ event Sweep(address indexed caller, uint256 assets); - /** - * @notice Emitted when the `_lastRecordedExchangeRate` is updated. - * @param exchangeRate The recorded exchange rate - * @dev This happens on mint and burn of shares - */ - event RecordedExchangeRate(uint256 exchangeRate); - /* ============ Variables ============ */ /// @notice Address of the TwabController used to keep track of balances. @@ -270,9 +276,6 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { /// @notice Underlying asset unit (i.e. 10 ** 18 for DAI). uint256 private _assetUnit; - /// @notice Most recent exchange rate recorded when burning or minting Vault shares. - uint256 private _lastRecordedExchangeRate; - /// @notice Yield fee percentage represented in integer format with 9 decimal places (i.e. 10000000 = 0.01 = 1%). uint256 private _yieldFeePercentage; @@ -410,12 +413,12 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { } /** - * @notice Current exchange rate between the Vault shares and - * the total amount of underlying assets withdrawable from the YieldVault. + * @notice Current exchange rate between the amount of underlying assets withdrawable from the YieldVault + * and the total supply of shares minted by this Vault. * @return uint256 Current exchange rate */ function exchangeRate() public view returns (uint256) { - return _currentExchangeRate(); + return _exchangeRate(); } /** @@ -480,6 +483,8 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { /// @inheritdoc ERC4626 function deposit(uint256 _assets, address _receiver) public virtual override returns (uint256) { + _requireVaultCollateralized(); + if (_assets > maxDeposit(_receiver)) revert DepositMoreThanMax(_receiver, _assets, maxDeposit(_receiver)); @@ -513,6 +518,8 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { /// @inheritdoc ERC4626 function mint(uint256 _shares, address _receiver) public virtual override returns (uint256) { + _requireVaultCollateralized(); + uint256 _assets = _convertToAssets(_shares, Math.Rounding.Up); _deposit(msg.sender, _receiver, _assets, _shares); @@ -911,49 +918,32 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { * @inheritdoc ERC4626 * @param _assets Amount of assets to convert * @param _rounding Rounding mode (i.e. down or up) - * @return uint256 Amount of shares for the assets + * @return uint256 Amount of shares corresponding to the assets */ function _convertToShares( uint256 _assets, Math.Rounding _rounding ) internal view virtual override returns (uint256) { - uint256 _exchangeRate = _currentExchangeRate(); - return - (_assets == 0 || _exchangeRate == 0) + (_assets == 0 || _totalSupply() == 0) ? _assets - : _assets.mulDiv(_assetUnit, _exchangeRate, _rounding); + : _assets.mulDiv(_assetUnit, _exchangeRate(), _rounding); } /** * @inheritdoc ERC4626 * @param _shares Amount of shares to convert * @param _rounding Rounding mode (i.e. down or up) - * @return uint256 Amount of assets represented by the shares + * @return uint256 Amount of assets corresponding to the shares */ function _convertToAssets( uint256 _shares, Math.Rounding _rounding ) internal view virtual override returns (uint256) { - return _convertToAssets(_shares, _currentExchangeRate(), _rounding); - } - - /** - * @notice Convert `_shares` to `_assets`. - * @param _shares Amount of shares to convert - * @param _exchangeRate Exchange rate used to convert `_shares` - * @param _rounding Rounding mode (i.e. down or up) - * @return uint256 Amount of assets represented by the shares - */ - function _convertToAssets( - uint256 _shares, - uint256 _exchangeRate, - Math.Rounding _rounding - ) internal view returns (uint256) { return - (_shares == 0 || _exchangeRate == 0) + (_shares == 0 || _totalSupply() == 0) ? _shares - : _shares.mulDiv(_exchangeRate, _assetUnit, _rounding); + : _shares.mulDiv(_exchangeRate(), _assetUnit, _rounding); } /* ============ Deposit Functions ============ */ @@ -1011,7 +1001,16 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { ); } + uint256 _withdrawableAssetsBefore = _yieldVault.maxWithdraw(address(this)); + _yieldVault.deposit(_assets, address(this)); + + uint256 _expectedWithdrawableAssets = _withdrawableAssetsBefore + _assets; + uint256 _withdrawableAssetsAfter = _yieldVault.maxWithdraw(address(this)); + + if (_withdrawableAssetsAfter < _expectedWithdrawableAssets) + revert YVWithdrawableAssetsLTExpected(_withdrawableAssetsAfter, _expectedWithdrawableAssets); + _mint(_receiver, _shares); emit Deposit(_caller, _receiver, _assets, _shares); @@ -1055,6 +1054,8 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { uint256 _assets, uint256 _shares ) internal virtual override { + if (_assets == 0) revert WithdrawZeroAssets(); + if (_caller != _owner) { _spendAllowance(_owner, _caller, _shares); } @@ -1070,8 +1071,6 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { _yieldVault.withdraw(_assets, address(this), address(this)); SafeERC20.safeTransfer(IERC20(asset()), _receiver, _assets); - _updateExchangeRate(); - emit Withdraw(_caller, _receiver, _owner, _assets, _shares); } @@ -1152,12 +1151,6 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { /* ============ State Functions ============ */ - /// @notice Update exchange rate with the current exchange rate. - function _updateExchangeRate() internal { - _lastRecordedExchangeRate = _currentExchangeRate(); - emit RecordedExchangeRate(_lastRecordedExchangeRate); - } - /** * @notice Creates `_shares` tokens and assigns them to `_receiver`, increasing the total supply. * @param _receiver Address that will receive the minted shares @@ -1171,7 +1164,6 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { revert MintMoreThanMax(_receiver, _shares, maxMint(_receiver)); _twabController.mint(_receiver, SafeCast.toUint96(_shares)); - _updateExchangeRate(); emit Transfer(address(0), _receiver, _shares); } @@ -1207,45 +1199,48 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { } /** - * @notice Calculate exchange rate between the amount of assets withdrawable from the YieldVault - * and the amount of shares minted by this Vault. - * @dev The amount of yield generated by the YieldVault is excluded from the calculation, - * so users can only withdraw the amount of underlying assets they deposited. - * Except when the vault is undercollateralized, in this case, any unclaimed yield is included. - * @dev We start with an exchange rate of 1 which is equal to 1 underlying asset unit. - * @return uint256 Exchange rate + * @notice Calculates the exchange rate between the amount of underlying assets withdrawable from the YieldVault + * and the total supply of shares minted by this Vault. + * @dev The initial exchange rate is 1, representing 1 unit of underlying asset. + * @dev When the Vault is collateralized, Vault shares are minted at a 1:1 ratio based on the user's deposited underlying assets. + * The total supply of shares corresponds directly to the total amount of underlying assets deposited into the YieldVault. + * Users have the ability to withdraw only the quantity of underlying assets they initially deposited, + * without access to any of the accumulated yield within the YieldVault. + * @dev When the Vault is undercollateralized, the exchange rate diminishes below 1. + * Withdrawals can be made by users for their corresponding deposit shares, + * and any remaining unclaimed yield is distributed proportionally among depositors. + * @return uint256 Current exchange rate */ - function _currentExchangeRate() internal view returns (uint256) { - uint256 _totalSupplyAmount = _totalSupply(); - uint256 _depositedAssets = _convertToAssets( - _totalSupplyAmount, - _lastRecordedExchangeRate, - Math.Rounding.Down - ); - + function _exchangeRate() internal view returns (uint256) { + uint256 _depositedAssets = _totalSupply(); uint256 _withdrawableAssets = _yieldVault.maxWithdraw(address(this)); - // If the Vault is collateralized, yield is excluded from the withdrawable amount - // and users can only withdraw the amount of underlying assets they deposited. - // Otherwise, any unclaimed yield is included and shared proportionally amongst depositors. - if (_withdrawableAssets > _depositedAssets) { - _withdrawableAssets = _depositedAssets; + // If the Vault is collateralized, users can only withdraw the amount of underlying assets they deposited. + // An exchange rate of 1 is returned to exclude the amount of yield generated by the YieldVault. + if (_withdrawableAssets >= _depositedAssets) { + return _assetUnit; } - if (_totalSupplyAmount != 0 && _withdrawableAssets != 0) { - return _withdrawableAssets.mulDiv(_assetUnit, _totalSupplyAmount, Math.Rounding.Down); + // Otherwise, users can withdraw their corresponding deposit shares and + // any unclaimed yield is factored in and distributed proportionally among depositors. + if (_depositedAssets != 0 && _withdrawableAssets != 0) { + return _withdrawableAssets.mulDiv(_assetUnit, _depositedAssets, Math.Rounding.Down); } - return _assetUnit; + // Case when `_withdrawableAssets == 0` but `_depositedAssets != 0`. + // The Vault is entirely undercollateralized + // or the YieldVault has paused withdrawals and shares can't be redeemed. + return 0; } /** * @notice Check if the Vault is collateralized. - * @dev The vault is collateralized if the exchange rate is greater than or equal to 1 underlying asset unit. + * @dev The vault is collateralized if the total amount of underlying assets currently held by the YieldVault + * is greater than or equal to the total supply of shares minted by the Vault. * @return bool True if the vault is collateralized, false otherwise */ function _isVaultCollateralized() internal view returns (bool) { - return _currentExchangeRate() >= _assetUnit; + return _yieldVault.maxWithdraw(address(this)) >= _totalSupply(); } /// @notice Require reverting if the vault is under-collateralized. diff --git a/test/contracts/mock/YieldVault.sol b/test/contracts/mock/YieldVault.sol index 8f34ee4..ee951da 100644 --- a/test/contracts/mock/YieldVault.sol +++ b/test/contracts/mock/YieldVault.sol @@ -10,10 +10,6 @@ contract YieldVault is ERC4626Mock { constructor(address _asset, string memory _name, string memory _symbol) ERC4626Mock(_asset) {} - function burnAssets(address _account, uint256 _assets) external { - ERC20Mock(asset()).burn(_account, _assets); - } - /** * We override the virtual shares and assets implementation since this approach captures * a very small part of the yield being accrued, which offsets by 1 wei @@ -26,24 +22,7 @@ contract YieldVault is ERC4626Mock { Math.Rounding rounding ) internal view virtual override returns (uint256) { uint256 supply = totalSupply(); - return - (assets == 0 || supply == 0) - ? _initialConvertToShares(assets, rounding) - : assets.mulDiv(supply, totalAssets(), rounding); - } - - function _initialConvertToShares( - uint256 assets, - Math.Rounding /*rounding*/ - ) internal view virtual returns (uint256 shares) { - return assets; - } - - function _initialConvertToAssets( - uint256 shares, - Math.Rounding /*rounding*/ - ) internal view virtual returns (uint256) { - return shares; + return (assets == 0 || supply == 0) ? assets : assets.mulDiv(supply, totalAssets(), rounding); } function _convertToAssets( @@ -51,9 +30,6 @@ contract YieldVault is ERC4626Mock { Math.Rounding rounding ) internal view virtual override returns (uint256) { uint256 supply = totalSupply(); - return - (supply == 0) - ? _initialConvertToAssets(shares, rounding) - : shares.mulDiv(totalAssets(), supply, rounding); + return (shares == 0 || supply == 0) ? shares : shares.mulDiv(totalAssets(), supply, rounding); } } diff --git a/test/unit/Vault/Deposit.feature b/test/unit/Vault/Deposit.feature index b3ee4bc..0bbe08d 100644 --- a/test/unit/Vault/Deposit.feature +++ b/test/unit/Vault/Deposit.feature @@ -61,12 +61,22 @@ Feature: Deposit Scenario: Alice deposits into the Vault Given Alice owns 0 Vault shares When Alice deposits type(uint96).max + 1 underlying assets - Then the transaction reverts with the custom error DepositMoreThanMax + Then the transaction reverts with the custom error `DepositMoreThanMax` Scenario: Alice deposits into the Vault Given Alice owns 0 Vault shares and YieldVault's maxDeposit function returns type(uint88).max When Alice deposits type(uint88).max + 1 underlying assets - Then the transaction reverts with the custom error DepositMoreThanMax + Then the transaction reverts with the custom error `DepositMoreThanMax` + + Scenario: Alice deposits into the Vault + Given Alice owns 0 Vault shares and the YieldVault's exchange rate has been manipulated + When Alice deposits 1,000 underlying assets + Then the transaction reverts with the custom error `YVWithdrawableAssetsLTExpected` + + Scenario: Alice deposits into the Vault + Given Alice owns 0 Vault shares and the Vault is undercollateralized + When Alice deposits 1,000 underlying assets + Then the transaction reverts with the custom error `VaultUnderCollateralized` # Deposit - Attacks # Inflation attack @@ -121,6 +131,11 @@ Feature: Deposit When Alice mints 0 shares Then the transaction reverts with the custom error MintZeroShares + Scenario: Alice mints 1,000 shares from the Vault + Given Alice owns 0 Vault shares and the Vault is undercollateralized + When Alice mints 1,000 shares + Then the transaction reverts with the custom error VaultUnderCollateralized + # Sponsor Scenario: Alice sponsors the Vault Given Alice owns 0 Vault shares and has not sponsored the Vault diff --git a/test/unit/Vault/Deposit.t.sol b/test/unit/Vault/Deposit.t.sol index 1e179d3..17355ac 100644 --- a/test/unit/Vault/Deposit.t.sol +++ b/test/unit/Vault/Deposit.t.sol @@ -5,7 +5,7 @@ import { BrokenToken } from "brokentoken/BrokenToken.sol"; import { IERC4626 } from "openzeppelin/token/ERC20/extensions/ERC4626.sol"; import { IERC20, UnitBaseSetup } from "../../utils/UnitBaseSetup.t.sol"; -import { MintMoreThanMax, MintZeroShares, DepositMoreThanMax, SweepZeroAssets } from "../../../src/Vault.sol"; +import "../../../src/Vault.sol"; contract VaultDepositTest is UnitBaseSetup, BrokenToken { /* ============ Events ============ */ @@ -17,8 +17,6 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken { event Transfer(address indexed from, address indexed to, uint256 value); - event RecordedExchangeRate(uint256 exchangeRate); - /* ============ Tests ============ */ /* ============ Deposit ============ */ @@ -29,9 +27,6 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken { underlyingAsset.mint(alice, _amount); underlyingAsset.approve(address(vault), type(uint256).max); - vm.expectEmit(); - emit RecordedExchangeRate(1e18); - vm.expectEmit(); emit Transfer(address(0), alice, _amount); @@ -208,6 +203,60 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken { vm.expectRevert( abi.encodeWithSelector(DepositMoreThanMax.selector, alice, _amount, type(uint88).max) ); + + vault.deposit(_amount, alice); + + vm.stopPrank(); + } + + function testYieldVaultExchangeRateManipulated() external { + vm.startPrank(alice); + + // Alice deposits in a new YieldVault + uint256 _yieldVaultAmount = 333e18; + + underlyingAsset.mint(alice, _yieldVaultAmount); + underlyingAsset.approve(address(yieldVault), type(uint256).max); + + yieldVault.deposit(_yieldVaultAmount, alice); + + // 0.1e18 underlying assets are sent to the YieldVault + // to manipulate the exchange rate + underlyingAsset.mint(address(yieldVault), 0.1e18); + + // When Alice deposits in the Vault, her deposit reverts + // because the amount of assets withdrawable from the YieldVault + // is lower than the amount deposited by Alice + uint256 _vaultAmount = 1000e18; + + underlyingAsset.mint(alice, _vaultAmount); + underlyingAsset.approve(address(vault), type(uint256).max); + + vm.expectRevert( + abi.encodeWithSelector( + YVWithdrawableAssetsLTExpected.selector, + _vaultAmount - 1, + _vaultAmount + ) + ); + + vault.deposit(_vaultAmount, alice); + } + + function testDepositVaultUndercollateralized() external { + vm.startPrank(alice); + + uint256 _amount = 1000; + + underlyingAsset.mint(alice, _amount); + underlyingAsset.approve(address(vault), type(uint256).max); + + vault.deposit(_amount, alice); + + underlyingAsset.burn(address(yieldVault), _amount); + + vm.expectRevert(abi.encodeWithSelector(VaultUnderCollateralized.selector)); + vault.deposit(_amount, alice); vm.stopPrank(); @@ -406,6 +455,25 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken { vm.stopPrank(); } + function testMintVaultUndercollateralized() external { + vm.startPrank(alice); + + uint256 _amount = 1000e18; + + underlyingAsset.mint(alice, _amount); + underlyingAsset.approve(address(vault), type(uint256).max); + + vault.mint(_amount, alice); + + underlyingAsset.burn(address(yieldVault), _amount); + + vm.expectRevert(abi.encodeWithSelector(VaultUnderCollateralized.selector)); + + vault.mint(_amount, alice); + + vm.stopPrank(); + } + /* ============ Sponsor ============ */ function testSponsor() external { vm.startPrank(alice); diff --git a/test/unit/Vault/Undercollateralization.feature b/test/unit/Vault/Undercollateralization.feature index 88dca04..1611b4b 100644 --- a/test/unit/Vault/Undercollateralization.feature +++ b/test/unit/Vault/Undercollateralization.feature @@ -6,6 +6,22 @@ Feature: Undercollateralization Then Alice can only withdraw 7,500,000 underlying assets Then Bob can only withdraw 2,500,000 underlying assets + Scenario: The YieldVault loses underlying assets and is now undercollateralized + Given Alice owns 15,000,000 Vault shares and Bob owns 5,000,000 Vault shares + When the YieldVault loses 10,000,000 underlying assets and gain them back + Then `isVaultCollateralized` must return false and then true + Then Bob only withdraw 2,500,000 underlying assets + Then Alice withdraw her 10,000,000 underlying assets cause she waited for the YieldVault to be re-collateralized + + Scenario: The YieldVault loses 100% of his underlying assets and is now entirely undercollateralized + Given Alice owns 15,000,000 Vault shares and Bob owns 5,000,000 Vault shares + When the YieldVault loses 20,000,000 underlying assets + Then `isVaultCollateralized` must return false + Then `exchangeRate` must return 0 + Then Bob can't withdraw his 2,500,000 underlying assets + Then Alice can't withdraw her 10,000,000 underlying assets + Then Alice can't deposit into the Vault + # With yield Scenario: The YieldVault loses underlying assets and is now undercollateralized Given Alice owns 15,000,000 Vault shares, Bob owns 5,000,000 Vault shares and the Vault has accrued 400,000 in yield diff --git a/test/unit/Vault/Undercollateralization.t.sol b/test/unit/Vault/Undercollateralization.t.sol index becda8a..30f5bb5 100644 --- a/test/unit/Vault/Undercollateralization.t.sol +++ b/test/unit/Vault/Undercollateralization.t.sol @@ -5,10 +5,6 @@ import { UnitBaseSetup } from "../../utils/UnitBaseSetup.t.sol"; import "../../../src/Vault.sol"; contract VaultUndercollateralizationTest is UnitBaseSetup { - /* ============ Events ============ */ - - event RecordedExchangeRate(uint256 exchangeRate); - /* ============ Tests ============ */ /* ============ Undercollateralization without yield fees accrued ============ */ @@ -41,6 +37,7 @@ contract VaultUndercollateralizationTest is UnitBaseSetup { underlyingAsset.burn(address(yieldVault), 10_000_000e18); assertEq(vault.isVaultCollateralized(), false); + assertEq(vault.exchangeRate(), 5e17); // 50% assertEq(vault.maxWithdraw(alice), _aliceAmountUndercollateralized); assertEq(vault.maxWithdraw(bob), _bobAmountUndercollateralized); @@ -55,6 +52,7 @@ contract VaultUndercollateralizationTest is UnitBaseSetup { vm.stopPrank(); assertEq(vault.isVaultCollateralized(), false); + assertEq(vault.exchangeRate(), 5e17); // 50% vm.startPrank(bob); @@ -66,39 +64,127 @@ contract VaultUndercollateralizationTest is UnitBaseSetup { // The Vault is now back to his initial state with no more shares, // so the exchange rate is reset and the Vault is no longer undercollateralized assertEq(vault.isVaultCollateralized(), true); + assertEq(vault.exchangeRate(), 1e18); // 100% assertEq(vault.totalSupply(), 0); } - /* ============ Undercollateralization exchange rate reset ============ */ - function testUndercollateralizationExchangeRateReset() external { - uint256 _aliceAmount = 20_000_000e18; - uint256 _aliceAmountUndercollateralized = 10_000_000e18; + function testUndercollateralizationYieldVaultReCollateralized() external { + uint256 _aliceAmount = 15_000_000e18; underlyingAsset.mint(alice, _aliceAmount); + uint256 _bobAmount = 5_000_000e18; + uint256 _bobUndercollateralizedAmount = 2_500_000e18; + + underlyingAsset.mint(bob, _bobAmount); + vm.startPrank(alice); _deposit(underlyingAsset, vault, _aliceAmount, alice); assertEq(vault.balanceOf(alice), _aliceAmount); + vm.stopPrank(); + + vm.startPrank(bob); + + _deposit(underlyingAsset, vault, _bobAmount, bob); + assertEq(vault.balanceOf(bob), _bobAmount); + assertEq(vault.isVaultCollateralized(), true); // We burn underlying assets from the YieldVault to trigger the undercollateralization - underlyingAsset.burn(address(yieldVault), 10_000_000e18); + uint256 _undercollateralizedAmount = 10_000_000e18; + underlyingAsset.burn(address(yieldVault), _undercollateralizedAmount); - assertEq(vault.exchangeRate(), 5e17); // 50% assertEq(vault.isVaultCollateralized(), false); + assertEq(vault.exchangeRate(), 5e17); // 50% - assertEq(vault.maxWithdraw(alice), _aliceAmountUndercollateralized); + // Bob decides to take the loss and withdraw his shares of the deposit + assertEq(vault.maxWithdraw(bob), _bobUndercollateralizedAmount); - // After the next withdraw, there will be no assets left in the vault, so the exchange rate should be reset after the shares are burned - vm.expectEmit(); - emit RecordedExchangeRate(1e18); // 100% + vault.withdraw(vault.maxWithdraw(bob), bob, bob); + assertEq(underlyingAsset.balanceOf(bob), _bobUndercollateralizedAmount); + + vm.stopPrank(); + + // Funds are returned to the YieldVault + underlyingAsset.mint(address(yieldVault), _undercollateralizedAmount); + + assertEq(vault.isVaultCollateralized(), true); + assertEq(vault.exchangeRate(), 1e18); // 100% + + vm.startPrank(alice); + + // Alice decided to wait and can now withdraw her full amount + assertEq(vault.maxWithdraw(alice), _aliceAmount); vault.withdraw(vault.maxWithdraw(alice), alice, alice); + assertEq(underlyingAsset.balanceOf(alice), _aliceAmount); - assertEq(underlyingAsset.balanceOf(alice), _aliceAmountUndercollateralized); + vm.stopPrank(); + + assertEq(vault.isVaultCollateralized(), true); assertEq(vault.totalSupply(), 0); + } + + function testUndercollateralizationYieldVaultEmpty() external { + uint256 _aliceAmount = 15_000_000e18; + + underlyingAsset.mint(alice, _aliceAmount); + + uint256 _bobAmount = 5_000_000e18; + + underlyingAsset.mint(bob, _bobAmount); + + vm.startPrank(alice); + + _deposit(underlyingAsset, vault, _aliceAmount, alice); + assertEq(vault.balanceOf(alice), _aliceAmount); + + vm.stopPrank(); + + vm.startPrank(bob); + + _deposit(underlyingAsset, vault, _bobAmount, bob); + assertEq(vault.balanceOf(bob), _bobAmount); + + assertEq(vault.isVaultCollateralized(), true); + + // We burn all the underlying assets from the YieldVault to trigger the undercollateralization + uint256 _undercollateralizedAmount = 20_000_000e18; + underlyingAsset.burn(address(yieldVault), _undercollateralizedAmount); + + assertEq(vault.isVaultCollateralized(), false); + assertEq(vault.exchangeRate(), 0); + + uint256 _bobMaxWithdraw = vault.maxWithdraw(bob); + + // Bob can't withdraw any assets + assertEq(_bobMaxWithdraw, 0); + + vm.expectRevert(abi.encodeWithSelector(WithdrawZeroAssets.selector)); + + vault.withdraw(_bobMaxWithdraw, bob, bob); + + vm.stopPrank(); + + vm.startPrank(alice); + + uint256 _aliceMaxWithdraw = vault.maxWithdraw(alice); + + // Alice can't withdraw any assets + assertEq(_aliceMaxWithdraw, 0); + + vm.expectRevert(abi.encodeWithSelector(WithdrawZeroAssets.selector)); + + vault.withdraw(_aliceMaxWithdraw, alice, alice); + + underlyingAsset.mint(alice, _aliceAmount); + + // Alice can't deposit into an undercollateralized vault + vm.expectRevert(abi.encodeWithSelector(VaultUnderCollateralized.selector)); + + vault.deposit(_aliceAmount, alice); vm.stopPrank(); } diff --git a/test/unit/Vault/Withdraw.feature b/test/unit/Vault/Withdraw.feature index 7983da2..fbf9e9e 100644 --- a/test/unit/Vault/Withdraw.feature +++ b/test/unit/Vault/Withdraw.feature @@ -43,6 +43,17 @@ Feature: Withdraw Then the YieldVault balance of underlying assets must be equal to 0 Then the Vault `totalSupply` must be equal to 0 + # Withdraw - Errors + Scenario: Alice withdraws 1,001 underlying assets + Given Alice owns 1,000 Vault shares + When Alice `withdraw` 1,001 underlying assets + Then the transaction reverts with the custom error `WithdrawMoreThanMax` + + Scenario: Alice withdraws 0 underlying assets + Given Alice owns 0 Vault shares + When Alice `withdraw` 0 underlying assets + Then the transaction reverts with the custom error `WithdrawZeroAssets` + # Redeem Scenario: Alice redeems her full deposit Given Alice owns 1,000 Vault shares @@ -87,3 +98,14 @@ Feature: Withdraw Then the Vault balance of YieldVault shares must be equal to 0 Then the YieldVault balance of underlying assets must be equal to 0 Then the Vault `totalSupply` must be equal to 0 + + # Redeem - Errors + Scenario: Alice redeems 1,001 Vault shares + Given Alice owns 1,000 Vault shares + When Alice `redeem` 1,001 shares + Then the transaction reverts with the custom error `RedeemMoreThanMax` + + Scenario: Alice redeems 0 shares + Given Alice owns 0 Vault shares + When Alice `redeem` 0 shares + Then the transaction reverts with the custom error `WithdrawZeroAssets` diff --git a/test/unit/Vault/Withdraw.t.sol b/test/unit/Vault/Withdraw.t.sol index 66ff63f..9032157 100644 --- a/test/unit/Vault/Withdraw.t.sol +++ b/test/unit/Vault/Withdraw.t.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.19; import { UnitBaseSetup, IERC20 } from "../../utils/UnitBaseSetup.t.sol"; -import { WithdrawMoreThanMax, RedeemMoreThanMax } from "../../../src/Vault.sol"; +import "../../../src/Vault.sol"; contract VaultWithdrawTest is UnitBaseSetup { /* ============ Events ============ */ @@ -47,21 +47,6 @@ contract VaultWithdrawTest is UnitBaseSetup { vm.stopPrank(); } - function testWithdrawMoreThanMax() external { - vm.startPrank(alice); - - uint256 _amount = 1000e18; - underlyingAsset.mint(alice, _amount); - _deposit(underlyingAsset, vault, _amount, alice); - - vm.expectRevert( - abi.encodeWithSelector(WithdrawMoreThanMax.selector, alice, _amount + 1, _amount) - ); - vault.withdraw(_amount + 1, alice, alice); - - vm.stopPrank(); - } - function testWithdrawHalfAmount() external { vm.startPrank(alice); @@ -248,7 +233,36 @@ contract VaultWithdrawTest is UnitBaseSetup { vm.stopPrank(); } + /* ============ Withdraw - Errors ============ */ + + function testWithdrawMoreThanMax() external { + vm.startPrank(alice); + + uint256 _amount = 1000e18; + underlyingAsset.mint(alice, _amount); + _deposit(underlyingAsset, vault, _amount, alice); + + vm.expectRevert( + abi.encodeWithSelector(WithdrawMoreThanMax.selector, alice, _amount + 1, _amount) + ); + + vault.withdraw(_amount + 1, alice, alice); + + vm.stopPrank(); + } + + function testWithdrawZeroAssets() external { + vm.startPrank(alice); + + vm.expectRevert(abi.encodeWithSelector(WithdrawZeroAssets.selector)); + + vault.withdraw(0, alice, alice); + + vm.stopPrank(); + } + /* ============ Redeem ============ */ + function testRedeemFullAmount() external { vm.startPrank(alice); @@ -278,21 +292,6 @@ contract VaultWithdrawTest is UnitBaseSetup { vm.stopPrank(); } - function testRedeemMoreThanMax() external { - vm.startPrank(alice); - - uint256 _amount = 1000e18; - underlyingAsset.mint(alice, _amount); - uint256 _shares = _deposit(underlyingAsset, vault, _amount, alice); - - vm.expectRevert( - abi.encodeWithSelector(RedeemMoreThanMax.selector, alice, _shares + 1, _shares) - ); - vault.redeem(_shares + 1, alice, alice); - - vm.stopPrank(); - } - function testRedeemHalfAmount() external { vm.startPrank(alice); @@ -392,4 +391,31 @@ contract VaultWithdrawTest is UnitBaseSetup { vm.stopPrank(); } + + /* ============ Redeem - Errors ============ */ + + function testRedeemMoreThanMax() external { + vm.startPrank(alice); + + uint256 _amount = 1000e18; + underlyingAsset.mint(alice, _amount); + uint256 _shares = _deposit(underlyingAsset, vault, _amount, alice); + + vm.expectRevert( + abi.encodeWithSelector(RedeemMoreThanMax.selector, alice, _shares + 1, _shares) + ); + vault.redeem(_shares + 1, alice, alice); + + vm.stopPrank(); + } + + function testRedeemZeroShares() external { + vm.startPrank(alice); + + vm.expectRevert(abi.encodeWithSelector(WithdrawZeroAssets.selector)); + + vault.redeem(0, alice, alice); + + vm.stopPrank(); + } } From 81fe36a5c670947219d63287f4e205cc7d2593f6 Mon Sep 17 00:00:00 2001 From: Pierrick Turelier Date: Tue, 15 Aug 2023 21:43:59 -0500 Subject: [PATCH 2/2] fix(exchangeRate): replace by collateral --- lcov.info | 513 +++++++++--------- src/Vault.sol | 67 +-- test/unit/Vault/Deposit.t.sol | 9 +- .../unit/Vault/Undercollateralization.feature | 1 - test/unit/Vault/Undercollateralization.t.sol | 30 +- test/unit/Vault/Vault.t.sol | 1 - test/unit/Vault/Withdraw.t.sol | 11 +- 7 files changed, 300 insertions(+), 332 deletions(-) diff --git a/lcov.info b/lcov.info index 615a3c9..c70fa57 100644 --- a/lcov.info +++ b/lcov.info @@ -1,71 +1,70 @@ TN: SF:src/Vault.sol -FN:1025,Vault._sponsor -FN:1050,Vault._withdraw -FN:1088,Vault._claimPrize -FN:1139,Vault._permit -FN:1162,Vault._mint -FN:1180,Vault._burn -FN:1195,Vault._transfer -FN:1214,Vault._exchangeRate -FN:1241,Vault._isVaultCollateralized -FN:1246,Vault._requireVaultCollateralized -FN:1256,Vault._setClaimer -FN:1265,Vault._setYieldFeePercentage -FN:1276,Vault._setYieldFeeRecipient -FN:369,Vault.availableYieldBalance -FN:383,Vault.availableYieldFeeBalance -FN:394,Vault.balanceOf -FN:401,Vault.decimals -FN:406,Vault.totalAssets -FN:411,Vault.totalSupply -FN:420,Vault.exchangeRate -FN:428,Vault.isVaultCollateralized -FN:436,Vault.maxDeposit -FN:450,Vault.maxMint -FN:466,Vault.mintYieldFee -FN:485,Vault.deposit -FN:507,Vault.depositWithPermit -FN:520,Vault.mint -FN:536,Vault.sponsor -FN:550,Vault.sponsorWithPermit -FN:567,Vault.sweep -FN:581,Vault.withdraw -FN:596,Vault.redeem -FN:612,Vault.liquidatableBalanceOf -FN:622,Vault.liquidate -FN:662,Vault.targetOf -FN:678,Vault.claimPrizes -FN:712,Vault.setClaimer -FN:724,Vault.setHooks -FN:736,Vault.setLiquidationPair -FN:762,Vault.setYieldFeePercentage -FN:775,Vault.setYieldFeeRecipient -FN:790,Vault.yieldFeeRecipient -FN:799,Vault.yieldFeePercentage -FN:808,Vault.yieldFeeShares -FN:816,Vault.twabController -FN:824,Vault.yieldVault -FN:832,Vault.liquidationPair -FN:840,Vault.prizePool -FN:848,Vault.claimer -FN:857,Vault.getHooks -FN:869,Vault._totalAssets -FN:877,Vault._totalSupply -FN:888,Vault._liquidatableBalanceOf -FN:903,Vault._availableYieldFeeBalance -FN:911,Vault._increaseYieldFeeBalance -FN:923,Vault._convertToShares -FN:939,Vault._convertToAssets -FN:968,Vault._deposit +FN:1019,Vault._sponsor +FN:1044,Vault._withdraw +FN:1082,Vault._claimPrize +FN:1133,Vault._permit +FN:1155,Vault._mint +FN:1172,Vault._burn +FN:1187,Vault._transfer +FN:1203,Vault._collateral +FN:1223,Vault._isVaultCollateralized +FN:1228,Vault._requireVaultCollateralized +FN:1238,Vault._setClaimer +FN:1247,Vault._setYieldFeePercentage +FN:1258,Vault._setYieldFeeRecipient +FN:364,Vault.availableYieldBalance +FN:378,Vault.availableYieldFeeBalance +FN:389,Vault.balanceOf +FN:396,Vault.decimals +FN:401,Vault.totalAssets +FN:406,Vault.totalSupply +FN:414,Vault.isVaultCollateralized +FN:422,Vault.maxDeposit +FN:436,Vault.maxMint +FN:452,Vault.mintYieldFee +FN:471,Vault.deposit +FN:493,Vault.depositWithPermit +FN:506,Vault.mint +FN:522,Vault.sponsor +FN:536,Vault.sponsorWithPermit +FN:553,Vault.sweep +FN:567,Vault.withdraw +FN:582,Vault.redeem +FN:598,Vault.liquidatableBalanceOf +FN:608,Vault.liquidate +FN:648,Vault.targetOf +FN:664,Vault.claimPrizes +FN:698,Vault.setClaimer +FN:710,Vault.setHooks +FN:722,Vault.setLiquidationPair +FN:748,Vault.setYieldFeePercentage +FN:761,Vault.setYieldFeeRecipient +FN:776,Vault.yieldFeeRecipient +FN:785,Vault.yieldFeePercentage +FN:794,Vault.yieldFeeShares +FN:802,Vault.twabController +FN:810,Vault.yieldVault +FN:818,Vault.liquidationPair +FN:826,Vault.prizePool +FN:834,Vault.claimer +FN:843,Vault.getHooks +FN:855,Vault._totalAssets +FN:863,Vault._totalSupply +FN:874,Vault._liquidatableBalanceOf +FN:889,Vault._availableYieldFeeBalance +FN:897,Vault._increaseYieldFeeBalance +FN:909,Vault._convertToShares +FN:929,Vault._convertToAssets +FN:962,Vault._deposit FNDA:16,Vault._sponsor -FNDA:3377,Vault._withdraw +FNDA:3391,Vault._withdraw FNDA:3,Vault._claimPrize FNDA:4,Vault._permit FNDA:33121,Vault._mint FNDA:3108,Vault._burn FNDA:258,Vault._transfer -FNDA:36878,Vault._exchangeRate +FNDA:74212,Vault._collateral FNDA:98111,Vault._isVaultCollateralized FNDA:33138,Vault._requireVaultCollateralized FNDA:2,Vault._setClaimer @@ -77,7 +76,6 @@ FNDA:3671,Vault.balanceOf FNDA:1,Vault.decimals FNDA:1024,Vault.totalAssets FNDA:2342,Vault.totalSupply -FNDA:8,Vault.exchangeRate FNDA:16,Vault.isVaultCollateralized FNDA:259,Vault.maxDeposit FNDA:256,Vault.maxMint @@ -88,8 +86,8 @@ FNDA:1543,Vault.mint FNDA:14,Vault.sponsor FNDA:2,Vault.sponsorWithPermit FNDA:2,Vault.sweep -FNDA:1828,Vault.withdraw -FNDA:1798,Vault.redeem +FNDA:1827,Vault.withdraw +FNDA:1799,Vault.redeem FNDA:1318,Vault.liquidatableBalanceOf FNDA:274,Vault.liquidate FNDA:1,Vault.targetOf @@ -109,206 +107,209 @@ FNDA:2,Vault.prizePool FNDA:3,Vault.claimer FNDA:1,Vault.getHooks FNDA:2630,Vault._totalAssets -FNDA:183314,Vault._totalSupply +FNDA:250487,Vault._totalSupply FNDA:1587,Vault._liquidatableBalanceOf FNDA:1589,Vault._availableYieldFeeBalance FNDA:9,Vault._increaseYieldFeeBalance -FNDA:34029,Vault._convertToShares -FNDA:40186,Vault._convertToAssets +FNDA:34046,Vault._convertToShares +FNDA:40166,Vault._convertToAssets FNDA:32853,Vault._deposit -FNF:58 -FNH:58 -DA:370,1606 +FNF:57 +FNH:57 +DA:365,1606 +DA:366,1606 DA:371,1606 -DA:376,1606 -DA:384,9 -DA:386,9 -DA:387,2 -DA:390,7 -DA:397,72989 -DA:402,1 -DA:407,1024 -DA:412,2342 -DA:421,8 -DA:429,16 -DA:437,31576 -DA:439,31532 -DA:440,31532 -DA:441,31532 -DA:443,31532 -DA:451,33381 -DA:453,33345 -DA:454,33345 -DA:456,33345 -DA:467,5 -DA:469,4 -DA:470,4 -DA:471,4 -DA:473,4 -DA:474,3 -DA:476,2 -DA:477,2 -DA:479,1 -DA:486,31316 -DA:488,31314 -DA:489,3 -DA:491,31311 -DA:492,31311 -DA:494,31310 -DA:515,2 -DA:516,2 -DA:521,1543 -DA:523,1542 -DA:525,1542 -DA:527,1539 -DA:537,14 -DA:558,2 -DA:559,2 -DA:568,2 -DA:569,2 -DA:571,1 -DA:573,1 -DA:575,1 -DA:586,1828 -DA:587,134 -DA:589,1694 -DA:590,1694 -DA:592,1568 -DA:601,1798 -DA:603,1683 -DA:604,1683 -DA:606,1540 -DA:613,1318 -DA:629,274 -DA:631,273 +DA:379,9 +DA:381,9 +DA:382,2 +DA:385,7 +DA:392,72968 +DA:397,1 +DA:402,1024 +DA:407,2342 +DA:415,16 +DA:423,31576 +DA:425,31535 +DA:426,31535 +DA:427,31535 +DA:429,31535 +DA:437,33381 +DA:439,33335 +DA:440,33335 +DA:442,33335 +DA:453,5 +DA:455,4 +DA:456,4 +DA:457,4 +DA:459,4 +DA:460,3 +DA:462,2 +DA:463,2 +DA:465,1 +DA:472,31316 +DA:474,31314 +DA:475,3 +DA:477,31311 +DA:478,31311 +DA:480,31310 +DA:501,2 +DA:502,2 +DA:507,1543 +DA:509,1542 +DA:511,1542 +DA:513,1539 +DA:523,14 +DA:544,2 +DA:545,2 +DA:554,2 +DA:555,2 +DA:557,1 +DA:559,1 +DA:561,1 +DA:572,1827 +DA:573,116 +DA:575,1711 +DA:576,1711 +DA:578,1567 +DA:587,1799 +DA:589,1680 +DA:590,1680 +DA:592,1541 +DA:599,1318 +DA:615,274 +DA:617,273 +DA:618,1 +DA:620,272 +DA:621,1 +DA:623,271 +DA:624,1 +DA:626,270 +DA:628,269 +DA:629,269 +DA:631,269 DA:632,1 -DA:634,272 -DA:635,1 -DA:637,271 -DA:638,1 -DA:640,270 -DA:642,269 -DA:643,269 -DA:645,269 -DA:646,1 -DA:648,268 -DA:650,268 -DA:651,9 -DA:656,268 -DA:658,267 -DA:663,1 -DA:685,5 -DA:687,3 -DA:689,3 -DA:690,3 -DA:691,3 -DA:692,3 -DA:702,3 -DA:713,2 -DA:714,2 -DA:716,2 -DA:717,2 -DA:725,3 -DA:727,3 -DA:739,280 -DA:741,279 -DA:742,279 -DA:744,279 -DA:745,1 -DA:748,279 -DA:750,279 -DA:752,279 -DA:753,279 -DA:763,13 -DA:764,13 -DA:766,12 -DA:767,12 -DA:776,5 -DA:777,5 -DA:779,5 -DA:780,5 -DA:791,2 -DA:800,2 -DA:809,7 -DA:817,2 -DA:825,2 -DA:833,2 -DA:841,2 -DA:849,3 -DA:858,1 -DA:870,2630 -DA:878,183314 -DA:889,1587 -DA:891,1587 -DA:894,1587 -DA:904,1589 -DA:912,9 -DA:927,34029 -DA:928,34029 -DA:943,40186 -DA:944,40186 -DA:974,32853 -DA:976,32852 -DA:977,32852 -DA:987,32852 -DA:988,32852 -DA:991,32852 -DA:992,1 -DA:996,32852 -DA:1004,32852 -DA:1006,32852 -DA:1008,32852 -DA:1009,32852 -DA:1011,32852 -DA:1012,1 -DA:1014,32851 -DA:1016,32849 -DA:1026,16 -DA:1029,16 -DA:1031,15 -DA:1034,16 -DA:1036,16 -DA:1057,3377 -DA:1059,3369 -DA:1060,1287 -DA:1069,3108 -DA:1071,3108 -DA:1072,3108 -DA:1074,3108 -DA:1095,3 -DA:1096,3 +DA:634,268 +DA:636,268 +DA:637,9 +DA:642,268 +DA:644,267 +DA:649,1 +DA:671,5 +DA:673,3 +DA:675,3 +DA:676,3 +DA:677,3 +DA:678,3 +DA:688,3 +DA:699,2 +DA:700,2 +DA:702,2 +DA:703,2 +DA:711,3 +DA:713,3 +DA:725,280 +DA:727,279 +DA:728,279 +DA:730,279 +DA:731,1 +DA:734,279 +DA:736,279 +DA:738,279 +DA:739,279 +DA:749,13 +DA:750,13 +DA:752,12 +DA:753,12 +DA:762,5 +DA:763,5 +DA:765,5 +DA:766,5 +DA:777,2 +DA:786,2 +DA:795,7 +DA:803,2 +DA:811,2 +DA:819,2 +DA:827,2 +DA:835,3 +DA:844,1 +DA:856,2630 +DA:864,250487 +DA:875,1587 +DA:877,1587 +DA:880,1587 +DA:890,1589 +DA:898,9 +DA:913,34046 +DA:914,34046 +DA:916,34046 +DA:917,7514 +DA:920,26532 +DA:933,40166 +DA:934,40166 +DA:936,40166 +DA:937,29828 +DA:940,10338 +DA:968,32853 +DA:970,32852 +DA:971,32852 +DA:981,32852 +DA:982,32852 +DA:985,32852 +DA:986,1 +DA:990,32852 +DA:998,32852 +DA:1000,32852 +DA:1002,32852 +DA:1003,32852 +DA:1005,32852 +DA:1006,1 +DA:1008,32851 +DA:1010,32849 +DA:1020,16 +DA:1023,16 +DA:1025,15 +DA:1028,16 +DA:1030,16 +DA:1051,3391 +DA:1053,3385 +DA:1054,1303 +DA:1063,3108 +DA:1065,3108 +DA:1066,3108 +DA:1068,3108 +DA:1089,3 +DA:1090,3 +DA:1092,3 +DA:1093,2 +DA:1095,1 DA:1098,3 -DA:1099,2 -DA:1101,1 -DA:1104,3 -DA:1113,3 -DA:1114,1 -DA:1123,3 -DA:1149,4 -DA:1163,33121 -DA:1164,4 -DA:1166,33117 -DA:1168,33117 -DA:1181,3108 -DA:1183,3108 -DA:1196,258 -DA:1198,258 -DA:1215,36878 -DA:1216,36878 -DA:1220,36878 -DA:1221,36085 -DA:1226,793 -DA:1227,788 -DA:1232,5 -DA:1242,98111 -DA:1247,33138 -DA:1257,2 -DA:1266,13 -DA:1267,1 -DA:1269,12 -DA:1277,5 -LF:189 -LH:189 +DA:1107,3 +DA:1108,1 +DA:1117,3 +DA:1143,4 +DA:1156,33121 +DA:1157,4 +DA:1159,33117 +DA:1161,33117 +DA:1173,3108 +DA:1175,3108 +DA:1188,258 +DA:1190,258 +DA:1204,74212 +DA:1205,74212 +DA:1208,74212 +DA:1209,73447 +DA:1214,765 +DA:1224,98111 +DA:1229,33138 +DA:1239,2 +DA:1248,13 +DA:1249,1 +DA:1251,12 +DA:1259,5 +LF:192 +LH:192 end_of_record TN: SF:src/VaultFactory.sol diff --git a/src/Vault.sol b/src/Vault.sol index ac132cd..4a70ccd 100644 --- a/src/Vault.sol +++ b/src/Vault.sol @@ -273,9 +273,6 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { /// @notice Address of the ILiquidationPair used to liquidate yield for prize token. ILiquidationPair private _liquidationPair; - /// @notice Underlying asset unit (i.e. 10 ** 18 for DAI). - uint256 private _assetUnit; - /// @notice Yield fee percentage represented in integer format with 9 decimal places (i.e. 10000000 = 0.01 = 1%). uint256 private _yieldFeePercentage; @@ -334,8 +331,6 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { _setYieldFeeRecipient(yieldFeeRecipient_); _setYieldFeePercentage(yieldFeePercentage_); - _assetUnit = 10 ** super.decimals(); - // Approve once for max amount asset_.safeApprove(address(yieldVault_), type(uint256).max); @@ -412,15 +407,6 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { return _totalSupply(); } - /** - * @notice Current exchange rate between the amount of underlying assets withdrawable from the YieldVault - * and the total supply of shares minted by this Vault. - * @return uint256 Current exchange rate - */ - function exchangeRate() public view returns (uint256) { - return _exchangeRate(); - } - /** * @notice Check if the Vault is collateralized. * @return bool True if the vault is collateralized, false otherwise @@ -924,10 +910,15 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { uint256 _assets, Math.Rounding _rounding ) internal view virtual override returns (uint256) { + uint256 _collateralAssets = _collateral(); + uint256 _depositedAssets = _totalSupply(); + + if (_assets == 0 || _depositedAssets == 0) { + return _assets; + } + return - (_assets == 0 || _totalSupply() == 0) - ? _assets - : _assets.mulDiv(_assetUnit, _exchangeRate(), _rounding); + _collateralAssets == 0 ? 0 : _assets.mulDiv(_depositedAssets, _collateralAssets, _rounding); } /** @@ -940,10 +931,15 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { uint256 _shares, Math.Rounding _rounding ) internal view virtual override returns (uint256) { + uint256 _collateralAssets = _collateral(); + uint256 _depositedAssets = _totalSupply(); + + if (_shares == 0 || _depositedAssets == 0) { + return _shares; + } + return - (_shares == 0 || _totalSupply() == 0) - ? _shares - : _shares.mulDiv(_exchangeRate(), _assetUnit, _rounding); + _collateralAssets == 0 ? 0 : _shares.mulDiv(_collateralAssets, _depositedAssets, _rounding); } /* ============ Deposit Functions ============ */ @@ -1157,7 +1153,6 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { * @param _shares Shares to mint * @dev Emits a {Transfer} event with `from` set to the zero address. * @dev `_receiver` cannot be the zero address. - * @dev Updates the exchange rate. */ function _mint(address _receiver, uint256 _shares) internal virtual override { if (_shares > maxMint(_receiver)) @@ -1175,7 +1170,6 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { * @dev Emits a {Transfer} event with `to` set to the zero address. * @dev `_owner` cannot be the zero address. * @dev `_owner` must have at least `_shares` tokens. - * @dev Updates the exchange rate. */ function _burn(address _owner, uint256 _shares) internal virtual override { _twabController.burn(_owner, SafeCast.toUint96(_shares)); @@ -1199,38 +1193,27 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable { } /** - * @notice Calculates the exchange rate between the amount of underlying assets withdrawable from the YieldVault - * and the total supply of shares minted by this Vault. - * @dev The initial exchange rate is 1, representing 1 unit of underlying asset. + * @notice Returns the quantity of withdrawable underlying assets held as collateral by the YieldVault. * @dev When the Vault is collateralized, Vault shares are minted at a 1:1 ratio based on the user's deposited underlying assets. * The total supply of shares corresponds directly to the total amount of underlying assets deposited into the YieldVault. * Users have the ability to withdraw only the quantity of underlying assets they initially deposited, * without access to any of the accumulated yield within the YieldVault. - * @dev When the Vault is undercollateralized, the exchange rate diminishes below 1. - * Withdrawals can be made by users for their corresponding deposit shares, - * and any remaining unclaimed yield is distributed proportionally among depositors. - * @return uint256 Current exchange rate + * @dev In case of undercollateralization, any remaining collateral within the YieldVault can be withdrawn. + * Withdrawals can be made by users for their corresponding deposit shares. + * @return uint256 Available collateral */ - function _exchangeRate() internal view returns (uint256) { + function _collateral() internal view returns (uint256) { uint256 _depositedAssets = _totalSupply(); uint256 _withdrawableAssets = _yieldVault.maxWithdraw(address(this)); // If the Vault is collateralized, users can only withdraw the amount of underlying assets they deposited. - // An exchange rate of 1 is returned to exclude the amount of yield generated by the YieldVault. if (_withdrawableAssets >= _depositedAssets) { - return _assetUnit; - } - - // Otherwise, users can withdraw their corresponding deposit shares and - // any unclaimed yield is factored in and distributed proportionally among depositors. - if (_depositedAssets != 0 && _withdrawableAssets != 0) { - return _withdrawableAssets.mulDiv(_assetUnit, _depositedAssets, Math.Rounding.Down); + return _depositedAssets; } - // Case when `_withdrawableAssets == 0` but `_depositedAssets != 0`. - // The Vault is entirely undercollateralized - // or the YieldVault has paused withdrawals and shares can't be redeemed. - return 0; + // Otherwise, any remaining collateral within the YieldVault is available + // and distributed proportionally among depositors. + return _withdrawableAssets; } /** diff --git a/test/unit/Vault/Deposit.t.sol b/test/unit/Vault/Deposit.t.sol index 17355ac..e444e11 100644 --- a/test/unit/Vault/Deposit.t.sol +++ b/test/unit/Vault/Deposit.t.sol @@ -306,14 +306,11 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken { */ assertEq(underlyingAsset.balanceOf(alice), _attackAmount); - // The exchange rate has not been manipulated and is still equal to 1 unit of asset - assertEq(vault.exchangeRate(), 10 ** 18); - /** * Alice receives back the same amount of Vault shares than underlying assets deposited * despite the attempt by Bob to inflate the exchange rate. - * This is because the exchange rate does not take into account - * the amount of underlying assets living in the Vault. + * This is because the Vault is collateralized + * and Alice can only withdraw the amount she deposited. */ assertEq(vault.balanceOf(alice), _amount); assertEq(IERC20(vault).balanceOf(alice), _amount); @@ -339,7 +336,7 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken { * Bob tries to redeem 1.99 shares to benefit from the attack * but it reverts cause he can only withdraw 1 wei of shares. */ - vault.redeem(vault.exchangeRate() * 2 - 1, bob, bob); + vault.redeem(2e18 - 1, bob, bob); assertEq(underlyingAsset.balanceOf(bob), 0); vm.stopPrank(); diff --git a/test/unit/Vault/Undercollateralization.feature b/test/unit/Vault/Undercollateralization.feature index 1611b4b..2505e72 100644 --- a/test/unit/Vault/Undercollateralization.feature +++ b/test/unit/Vault/Undercollateralization.feature @@ -17,7 +17,6 @@ Feature: Undercollateralization Given Alice owns 15,000,000 Vault shares and Bob owns 5,000,000 Vault shares When the YieldVault loses 20,000,000 underlying assets Then `isVaultCollateralized` must return false - Then `exchangeRate` must return 0 Then Bob can't withdraw his 2,500,000 underlying assets Then Alice can't withdraw her 10,000,000 underlying assets Then Alice can't deposit into the Vault diff --git a/test/unit/Vault/Undercollateralization.t.sol b/test/unit/Vault/Undercollateralization.t.sol index 30f5bb5..37892b1 100644 --- a/test/unit/Vault/Undercollateralization.t.sol +++ b/test/unit/Vault/Undercollateralization.t.sol @@ -37,7 +37,6 @@ contract VaultUndercollateralizationTest is UnitBaseSetup { underlyingAsset.burn(address(yieldVault), 10_000_000e18); assertEq(vault.isVaultCollateralized(), false); - assertEq(vault.exchangeRate(), 5e17); // 50% assertEq(vault.maxWithdraw(alice), _aliceAmountUndercollateralized); assertEq(vault.maxWithdraw(bob), _bobAmountUndercollateralized); @@ -52,7 +51,6 @@ contract VaultUndercollateralizationTest is UnitBaseSetup { vm.stopPrank(); assertEq(vault.isVaultCollateralized(), false); - assertEq(vault.exchangeRate(), 5e17); // 50% vm.startPrank(bob); @@ -61,10 +59,8 @@ contract VaultUndercollateralizationTest is UnitBaseSetup { vm.stopPrank(); - // The Vault is now back to his initial state with no more shares, - // so the exchange rate is reset and the Vault is no longer undercollateralized + // The Vault is now back to his initial state with no more shares assertEq(vault.isVaultCollateralized(), true); - assertEq(vault.exchangeRate(), 1e18); // 100% assertEq(vault.totalSupply(), 0); } @@ -97,7 +93,6 @@ contract VaultUndercollateralizationTest is UnitBaseSetup { underlyingAsset.burn(address(yieldVault), _undercollateralizedAmount); assertEq(vault.isVaultCollateralized(), false); - assertEq(vault.exchangeRate(), 5e17); // 50% // Bob decides to take the loss and withdraw his shares of the deposit assertEq(vault.maxWithdraw(bob), _bobUndercollateralizedAmount); @@ -111,7 +106,6 @@ contract VaultUndercollateralizationTest is UnitBaseSetup { underlyingAsset.mint(address(yieldVault), _undercollateralizedAmount); assertEq(vault.isVaultCollateralized(), true); - assertEq(vault.exchangeRate(), 1e18); // 100% vm.startPrank(alice); @@ -155,7 +149,6 @@ contract VaultUndercollateralizationTest is UnitBaseSetup { underlyingAsset.burn(address(yieldVault), _undercollateralizedAmount); assertEq(vault.isVaultCollateralized(), false); - assertEq(vault.exchangeRate(), 0); uint256 _bobMaxWithdraw = vault.maxWithdraw(bob); @@ -303,34 +296,35 @@ contract VaultUndercollateralizationTest is UnitBaseSetup { vm.startPrank(bob); - // We lose in precision because the Vault is undercollateralized - // and the exchange rate is below 1e18 and rounded down _bobAmount = _getMaxWithdraw(bob, vault, yieldVault); - assertApproxEqAbs(vault.maxWithdraw(bob), _bobAmount, 2382812); + assertEq(vault.maxWithdraw(bob), _bobAmount); vault.withdraw(vault.maxWithdraw(bob), bob, bob); - assertApproxEqAbs(underlyingAsset.balanceOf(bob), _bobAmount, 2382812); + assertEq(underlyingAsset.balanceOf(bob), _bobAmount); vm.stopPrank(); vm.startPrank(alice); _aliceAmount = _getMaxWithdraw(alice, vault, yieldVault); - assertApproxEqAbs(vault.maxWithdraw(alice), _aliceAmount, 2382812); + assertEq(vault.maxWithdraw(alice), _aliceAmount); - vault.withdraw(vault.maxWithdraw(alice), alice, alice); - assertApproxEqAbs(underlyingAsset.balanceOf(alice), _aliceAmount, 2382812); + // Due to the undercollateralization `maxWithdraw` rounds down + // and Alice would still own 1 Vault share after withdrawing + // We use `redeem` instead to withdraw the full amount and burn all shares + vault.redeem(vault.maxRedeem(alice), alice, alice); + assertEq(underlyingAsset.balanceOf(alice), _aliceAmount); vm.stopPrank(); uint256 _thisAmount = _getMaxWithdraw(address(this), vault, yieldVault); - assertApproxEqAbs(vault.maxWithdraw(address(this)), _thisAmount, 2440000); + assertEq(vault.maxWithdraw(address(this)), _thisAmount); vault.withdraw(vault.maxWithdraw(address(this)), address(this), address(this)); - assertApproxEqAbs(underlyingAsset.balanceOf(address(this)), _thisAmount, 2440000); + assertEq(underlyingAsset.balanceOf(address(this)), _thisAmount); assertEq(vault.totalSupply(), 0); - assertApproxEqAbs(underlyingAsset.balanceOf(address(yieldVault)), 0, 2440000); + assertEq(underlyingAsset.balanceOf(address(yieldVault)), 0); } function testPartialUndercollateralizationWithYieldFeesCaptured() external { diff --git a/test/unit/Vault/Vault.t.sol b/test/unit/Vault/Vault.t.sol index 83fdc7d..475aa63 100644 --- a/test/unit/Vault/Vault.t.sol +++ b/test/unit/Vault/Vault.t.sol @@ -67,7 +67,6 @@ contract VaultTest is UnitBaseSetup { assertEq(testVault.name(), vaultName); assertEq(testVault.symbol(), vaultSymbol); assertEq(testVault.decimals(), assetDecimals); - assertEq(testVault.exchangeRate(), 10 ** assetDecimals); assertEq(testVault.twabController(), address(twabController)); assertEq(testVault.yieldVault(), address(yieldVault)); assertEq(testVault.prizePool(), address(prizePool)); diff --git a/test/unit/Vault/Withdraw.t.sol b/test/unit/Vault/Withdraw.t.sol index 9032157..368935a 100644 --- a/test/unit/Vault/Withdraw.t.sol +++ b/test/unit/Vault/Withdraw.t.sol @@ -166,14 +166,9 @@ contract VaultWithdrawTest is UnitBaseSetup { // We burn the accrued yield to set the Vault in an undercollateralized state underlyingAsset.burn(address(yieldVault), _yield); - assertLt(vault.exchangeRate(), 1e18); - // The Vault is now undercollateralized, so users can withdraw their share of the deposits - // any unclaimed yield fee goes proportionally to each user - - // We lose a bit of precision due to the exchange rate being below 1e18 and rounded down - assertApproxEqAbs(vault.maxWithdraw(bob), _getMaxWithdraw(bob, vault, yieldVault), 6); - assertApproxEqAbs(vault.maxWithdraw(alice), _getMaxWithdraw(alice, vault, yieldVault), 693); + assertEq(vault.maxWithdraw(bob), _getMaxWithdraw(bob, vault, yieldVault)); + assertEq(vault.maxWithdraw(alice), _getMaxWithdraw(alice, vault, yieldVault)); vm.startPrank(alice); @@ -194,7 +189,7 @@ contract VaultWithdrawTest is UnitBaseSetup { assertEq(underlyingAsset.balanceOf(bob), _bobWithdrawableAmount); assertEq(vault.totalSupply(), 0); - assertApproxEqAbs(underlyingAsset.balanceOf(address(yieldVault)), 0, 7); + assertEq(underlyingAsset.balanceOf(address(yieldVault)), 0); vm.stopPrank(); }