From bf387ca8deda15428aa6080d2e25eccf9e31da79 Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Sun, 10 Sep 2023 17:13:23 +0200 Subject: [PATCH 1/4] test: draft --- contracts/mocks/IrmMock.sol | 12 +++++-- test/forge/BaseTest.sol | 17 ++++++++-- test/forge/FeeTest.sol | 44 ++++++++++++++++++++++++++ test/forge/VaultSeveralMarketsTest.sol | 10 ------ 4 files changed, 67 insertions(+), 16 deletions(-) create mode 100644 test/forge/FeeTest.sol diff --git a/contracts/mocks/IrmMock.sol b/contracts/mocks/IrmMock.sol index 53d07a0c..da1125c6 100644 --- a/contracts/mocks/IrmMock.sol +++ b/contracts/mocks/IrmMock.sol @@ -9,15 +9,21 @@ import {MathLib} from "@morpho-blue/libraries/MathLib.sol"; contract IrmMock is IIrm { using MathLib for uint128; - function borrowRateView(MarketParams memory, Market memory market) public pure returns (uint256) { + uint256 public rate; + + function setRate(uint256 newRate) external { + rate = newRate; + } + + function borrowRateView(MarketParams memory, Market memory market) public view returns (uint256) { uint256 utilization = market.totalBorrowAssets.wDivDown(market.totalSupplyAssets); // Divide by the number of seconds in a year. // This is a very simple model where x% utilization corresponds to x% APR. - return utilization / 365 days; + return rate == 0 ? utilization / 365 days : rate / 365 days; } - function borrowRate(MarketParams memory marketParams, Market memory market) external pure returns (uint256) { + function borrowRate(MarketParams memory marketParams, Market memory market) external view returns (uint256) { return borrowRateView(marketParams, market); } } diff --git a/test/forge/BaseTest.sol b/test/forge/BaseTest.sol index 11c5b62d..f4a71433 100644 --- a/test/forge/BaseTest.sol +++ b/test/forge/BaseTest.sol @@ -31,9 +31,10 @@ contract BaseTest is Test { address internal ONBEHALF = _addrFromHashedString("Morpho On Behalf"); address internal RECEIVER = _addrFromHashedString("Morpho Receiver"); address internal LIQUIDATOR = _addrFromHashedString("Morpho Liquidator"); - address internal OWNER = _addrFromHashedString("Morpho Owner"); - address internal RISK_MANAGER = _addrFromHashedString("Morpho Risk Manager"); - address internal ALLOCATOR = _addrFromHashedString("Morpho Allocator"); + address internal OWNER = _addrFromHashedString("Owner"); + address internal RISK_MANAGER = _addrFromHashedString("Risk Manager"); + address internal ALLOCATOR = _addrFromHashedString("Allocator"); + address internal FEE_RECIPIENT = _addrFromHashedString("Fee Recipient"); uint256 internal constant LLTV = 0.8 ether; uint256 internal constant TIMELOCK = 0; @@ -131,4 +132,14 @@ contract BaseTest is Test { vault.enableMarket(params.id()); vm.stopPrank(); } + + function _borrow(MarketParams memory params, uint256 amount) internal { + deal(address(collateralToken), BORROWER, type(uint256).max); + + vm.startPrank(BORROWER); + collateralToken.approve(address(morpho), type(uint256).max); + morpho.supplyCollateral(params, type(uint128).max, BORROWER, hex""); + morpho.borrow(params, amount, 0, BORROWER, BORROWER); + vm.stopPrank(); + } } diff --git a/test/forge/FeeTest.sol b/test/forge/FeeTest.sol new file mode 100644 index 00000000..f86d4c6d --- /dev/null +++ b/test/forge/FeeTest.sol @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +pragma solidity ^0.8.0; + +import "./BaseTest.sol"; + +contract FeeTest is BaseTest { + using MarketParamsLib for MarketParams; + + uint256 internal constant FEE = 0.1 ether; // 10% + + function setUp() public override { + super.setUp(); + + _submitAndEnableMarket(allMarkets[0], CAP); + + irm.setRate(0.1 ether); // 10% APY. + } + + function _setFee() internal { + vm.startPrank(OWNER); + vault.submitPendingFee(FEE); + vault.setFee(); + vault.setFeeRecipient(FEE_RECIPIENT); + vm.stopPrank(); + } + + function testShouldMintSharesToFeeRecipient(uint256 amount) public { + _setFee(); + + amount = bound(amount, MIN_TEST_AMOUNT, MAX_TEST_AMOUNT); + + vm.prank(SUPPLIER); + uint256 shares = vault.deposit(amount, SUPPLIER); + + _borrow(allMarkets[0], amount / 2); + + vm.warp(block.timestamp + 365 days); + + vm.prank(SUPPLIER); + vault.redeem(shares / 3, SUPPLIER, SUPPLIER); + + assertGt(vault.balanceOf(FEE_RECIPIENT), 0, "fee recipient balance is zero"); + } +} diff --git a/test/forge/VaultSeveralMarketsTest.sol b/test/forge/VaultSeveralMarketsTest.sol index 218f9f08..818c8601 100644 --- a/test/forge/VaultSeveralMarketsTest.sol +++ b/test/forge/VaultSeveralMarketsTest.sol @@ -202,16 +202,6 @@ contract VaultSeveralMarketsTest is BaseTest { _assertBalances(cap, amount - toWithdraw); } - function _borrow(MarketParams memory marketParams, uint256 amount) internal { - deal(address(collateralToken), BORROWER, type(uint256).max); - - vm.startPrank(BORROWER); - collateralToken.approve(address(morpho), type(uint256).max); - morpho.supplyCollateral(marketParams, type(uint128).max, BORROWER, hex""); - morpho.borrow(marketParams, amount, 0, BORROWER, BORROWER); - vm.stopPrank(); - } - function testWithdrawSeveralMarketsWithLessLiquidity(uint128 cap, uint256 toWithdraw, uint256 borrowed) public { cap = uint128(bound(cap, MIN_TEST_AMOUNT, MAX_TEST_AMOUNT)); uint256 amount = 3 * cap; From ebe58aa6c8b9e9198c63c50bdc5de7d64609e81b Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Mon, 11 Sep 2023 12:36:07 +0200 Subject: [PATCH 2/4] test: add simple tests --- contracts/SupplyVault.sol | 2 +- test/forge/BaseTest.sol | 2 ++ test/forge/FeeTest.sol | 52 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/contracts/SupplyVault.sol b/contracts/SupplyVault.sol index 0e6e2c74..a157dfee 100644 --- a/contracts/SupplyVault.sol +++ b/contracts/SupplyVault.sol @@ -56,7 +56,7 @@ contract SupplyVault is ERC4626, Ownable2Step, ISupplyVault { uint256 public timelock; /// @dev Stores the total assets owned by this vault when the fee was last accrued. - uint256 lastTotalAssets; + uint256 public lastTotalAssets; ConfigSet private _config; diff --git a/test/forge/BaseTest.sol b/test/forge/BaseTest.sol index f4a71433..382436cc 100644 --- a/test/forge/BaseTest.sol +++ b/test/forge/BaseTest.sol @@ -9,6 +9,8 @@ import {UtilsLib} from "@morpho-blue/libraries/UtilsLib.sol"; import {ERC20Mock as ERC20} from "contracts/mocks/ERC20Mock.sol"; import {OracleMock as Oracle} from "contracts/mocks/OracleMock.sol"; +import {WAD} from "@morpho-blue/libraries/MathLib.sol"; +import {Math} from "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; import {SupplyVault, IERC20, ErrorsLib, Pending, MarketAllocation} from "contracts/SupplyVault.sol"; import {Morpho, MarketParamsLib, MarketParams, SharesMathLib, Id} from "@morpho-blue/Morpho.sol"; diff --git a/test/forge/FeeTest.sol b/test/forge/FeeTest.sol index f86d4c6d..9c882ec6 100644 --- a/test/forge/FeeTest.sol +++ b/test/forge/FeeTest.sol @@ -4,9 +4,10 @@ pragma solidity ^0.8.0; import "./BaseTest.sol"; contract FeeTest is BaseTest { + using Math for uint256; using MarketParamsLib for MarketParams; - uint256 internal constant FEE = 0.1 ether; // 10% + uint256 constant internal FEE = 0.1 ether; // 10% function setUp() public override { super.setUp(); @@ -24,21 +25,64 @@ contract FeeTest is BaseTest { vm.stopPrank(); } - function testShouldMintSharesToFeeRecipient(uint256 amount) public { + function testLastTotalAssets(uint256 amount) public { + amount = bound(amount, MIN_TEST_AMOUNT, MAX_TEST_AMOUNT); + + _setFee(); + + assertEq(vault.lastTotalAssets(), 0); + + vm.prank(SUPPLIER); + vault.deposit(amount, SUPPLIER); + + // Update lastTotalAssets + vm.prank(SUPPLIER); + vault.withdraw(10, SUPPLIER, SUPPLIER); + + assertEq(vault.lastTotalAssets(), amount); + } + + function testAccounting() public { + uint256 amount = MAX_TEST_AMOUNT; + _setFee(); + assertEq(vault.lastTotalAssets(), 0); + + vm.prank(SUPPLIER); + vault.deposit(amount, SUPPLIER); + + // Update lastTotalAssets + vm.prank(SUPPLIER); + vault.withdraw(10, SUPPLIER, SUPPLIER); + + // supplier balance 9999999999999999999999999988 + // fee recipient balance 1111111111111111111111111111 + console2.log("supplier balance", vault.balanceOf(SUPPLIER)); + console2.log("fee recipient balance", vault.balanceOf(FEE_RECIPIENT)); + } + + function testShouldMintSharesToFeeRecipient(uint256 amount) public { amount = bound(amount, MIN_TEST_AMOUNT, MAX_TEST_AMOUNT); + _setFee(); + vm.prank(SUPPLIER); uint256 shares = vault.deposit(amount, SUPPLIER); - _borrow(allMarkets[0], amount / 2); + uint256 lastTotalAssets = vault.lastTotalAssets(); vm.warp(block.timestamp + 365 days); + uint256 totalAssetsAfter = vault.totalAssets(); + uint256 interest = totalAssetsAfter - lastTotalAssets; + uint256 feeAmount = interest.mulDiv(FEE, WAD); + uint256 feeShares = feeAmount.mulDiv(vault.totalSupply() + 1, totalAssetsAfter - feeAmount + 1, Math.Rounding.Down); + vm.prank(SUPPLIER); - vault.redeem(shares / 3, SUPPLIER, SUPPLIER); + vault.redeem(shares / 10, SUPPLIER, SUPPLIER); assertGt(vault.balanceOf(FEE_RECIPIENT), 0, "fee recipient balance is zero"); + assertEq(vault.balanceOf(FEE_RECIPIENT), feeShares, "fee recipient balance"); } } From 7a35159aa6e0395634443d68b7874b13accc13bb Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Mon, 11 Sep 2023 20:13:40 +0200 Subject: [PATCH 3/4] test: fee accrual --- test/forge/FeeTest.sol | 119 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 115 insertions(+), 4 deletions(-) diff --git a/test/forge/FeeTest.sol b/test/forge/FeeTest.sol index 9c882ec6..973e6e68 100644 --- a/test/forge/FeeTest.sol +++ b/test/forge/FeeTest.sol @@ -7,7 +7,7 @@ contract FeeTest is BaseTest { using Math for uint256; using MarketParamsLib for MarketParams; - uint256 constant internal FEE = 0.1 ether; // 10% + uint256 internal constant FEE = 0.1 ether; // 10% function setUp() public override { super.setUp(); @@ -77,12 +77,123 @@ contract FeeTest is BaseTest { uint256 totalAssetsAfter = vault.totalAssets(); uint256 interest = totalAssetsAfter - lastTotalAssets; uint256 feeAmount = interest.mulDiv(FEE, WAD); - uint256 feeShares = feeAmount.mulDiv(vault.totalSupply() + 1, totalAssetsAfter - feeAmount + 1, Math.Rounding.Down); + uint256 feeShares = + feeAmount.mulDiv(vault.totalSupply() + 1, totalAssetsAfter - feeAmount + 1, Math.Rounding.Down); vm.prank(SUPPLIER); - vault.redeem(shares / 10, SUPPLIER, SUPPLIER); + vault.redeem(shares / 10000, SUPPLIER, SUPPLIER); assertGt(vault.balanceOf(FEE_RECIPIENT), 0, "fee recipient balance is zero"); - assertEq(vault.balanceOf(FEE_RECIPIENT), feeShares, "fee recipient balance"); + assertEq(vault.balanceOf(FEE_RECIPIENT), feeShares, "vault.balanceOf(FEE_RECIPIENT)"); + assertApproxEqAbs( + vault.convertToAssets(vault.balanceOf(FEE_RECIPIENT)), + amount.mulDiv(FEE, WAD), + 1, + "fee recipient balance approx" + ); + } + + function testDepositShouldAccrueFee() public { + _setFee(); + + // Deposit to generate fees. + vm.prank(SUPPLIER); + vault.deposit(MAX_TEST_AMOUNT, SUPPLIER); + + assertEq(vault.balanceOf(FEE_RECIPIENT), 0); + + vm.warp(block.timestamp + 1); + + vm.prank(SUPPLIER); + vault.deposit(MAX_TEST_AMOUNT, SUPPLIER); + + assertGt(vault.balanceOf(FEE_RECIPIENT), 0); + } + + function testMintShouldAccrueFee() public { + _setFee(); + + // Deposit to generate fees. + vm.prank(SUPPLIER); + vault.deposit(MAX_TEST_AMOUNT, SUPPLIER); + + assertEq(vault.balanceOf(FEE_RECIPIENT), 0); + + vm.warp(block.timestamp + 1); + + vm.prank(SUPPLIER); + vault.mint(MAX_TEST_AMOUNT, SUPPLIER); + + assertGt(vault.balanceOf(FEE_RECIPIENT), 0); + } + + function testRedeemShouldAccrueFee() public { + _setFee(); + + // Deposit to generate fees. + vm.prank(SUPPLIER); + uint256 shares = vault.deposit(MAX_TEST_AMOUNT, SUPPLIER); + + assertEq(vault.balanceOf(FEE_RECIPIENT), 0); + + vm.warp(block.timestamp + 1); + + vm.prank(SUPPLIER); + vault.redeem(shares / 10, SUPPLIER, SUPPLIER); + + assertGt(vault.balanceOf(FEE_RECIPIENT), 0); + } + + function testWithdrawShouldAccrueFee() public { + _setFee(); + + // Deposit to generate fees. + vm.prank(SUPPLIER); + vault.deposit(MAX_TEST_AMOUNT, SUPPLIER); + + assertEq(vault.balanceOf(FEE_RECIPIENT), 0); + + vm.warp(block.timestamp + 1); + + vm.prank(SUPPLIER); + vault.redeem(MAX_TEST_AMOUNT / 10, SUPPLIER, SUPPLIER); + + assertGt(vault.balanceOf(FEE_RECIPIENT), 0); + } + + function testSetFeeShouldAccrueFee() public { + _setFee(); + + // Deposit to generate fees. + vm.prank(SUPPLIER); + vault.deposit(MAX_TEST_AMOUNT, SUPPLIER); + + assertEq(vault.balanceOf(FEE_RECIPIENT), 0); + + vm.warp(block.timestamp + 1); + + vm.startPrank(OWNER); + vault.submitPendingFee(0); + vault.setFee(); + vm.stopPrank(); + + assertGt(vault.balanceOf(FEE_RECIPIENT), 0); + } + + function testSetFeeRecipientShouldAccrueFee() public { + _setFee(); + + // Deposit to generate fees. + vm.prank(SUPPLIER); + vault.deposit(MAX_TEST_AMOUNT, SUPPLIER); + + assertEq(vault.balanceOf(FEE_RECIPIENT), 0); + + vm.warp(block.timestamp + 1); + + vm.prank(OWNER); + vault.setFeeRecipient(address(0)); + + assertGt(vault.balanceOf(FEE_RECIPIENT), 0); } } From b1127f5950c1750446f2483fde451a820e2a8521 Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Mon, 11 Sep 2023 21:18:26 +0200 Subject: [PATCH 4/4] fix: prevent several updated of lastTotalAssets in a block --- contracts/SupplyVault.sol | 6 +++++- test/forge/FeeTest.sol | 19 ++++++++----------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/contracts/SupplyVault.sol b/contracts/SupplyVault.sol index a157dfee..c9f822f9 100644 --- a/contracts/SupplyVault.sol +++ b/contracts/SupplyVault.sol @@ -57,6 +57,7 @@ contract SupplyVault is ERC4626, Ownable2Step, ISupplyVault { /// @dev Stores the total assets owned by this vault when the fee was last accrued. uint256 public lastTotalAssets; + uint256 public lastUpdateTimestamp; ConfigSet private _config; @@ -456,7 +457,10 @@ contract SupplyVault is ERC4626, Ownable2Step, ISupplyVault { } function _accrueFee() internal { - if (fee == 0 || feeRecipient == address(0)) return; + uint256 lastUpdate = lastUpdateTimestamp; + lastUpdateTimestamp = block.timestamp; + + if (lastUpdate == block.timestamp || fee == 0 || feeRecipient == address(0)) return; (uint256 newTotalAssets, uint256 feeShares) = _accruedFeeShares(); diff --git a/test/forge/FeeTest.sol b/test/forge/FeeTest.sol index 973e6e68..051ae328 100644 --- a/test/forge/FeeTest.sol +++ b/test/forge/FeeTest.sol @@ -25,41 +25,38 @@ contract FeeTest is BaseTest { vm.stopPrank(); } - function testLastTotalAssets(uint256 amount) public { + function testShouldNotUpdateLastTotalAssetsMoreThanOnce(uint256 amount) public { amount = bound(amount, MIN_TEST_AMOUNT, MAX_TEST_AMOUNT); _setFee(); - assertEq(vault.lastTotalAssets(), 0); + uint256 lastTotalAssets = vault.lastTotalAssets(); vm.prank(SUPPLIER); vault.deposit(amount, SUPPLIER); - // Update lastTotalAssets + // Try to update lastTotalAssets vm.prank(SUPPLIER); vault.withdraw(10, SUPPLIER, SUPPLIER); - assertEq(vault.lastTotalAssets(), amount); + assertEq(vault.lastTotalAssets(), lastTotalAssets); } - function testAccounting() public { + function testShouldNotIncreaseFeeRecipientBalanceWithingABlock() public { uint256 amount = MAX_TEST_AMOUNT; _setFee(); - assertEq(vault.lastTotalAssets(), 0); + uint256 feeRecipientBalance = vault.balanceOf(FEE_RECIPIENT); vm.prank(SUPPLIER); vault.deposit(amount, SUPPLIER); - // Update lastTotalAssets + // Try to update feeRecipientBalance vm.prank(SUPPLIER); vault.withdraw(10, SUPPLIER, SUPPLIER); - // supplier balance 9999999999999999999999999988 - // fee recipient balance 1111111111111111111111111111 - console2.log("supplier balance", vault.balanceOf(SUPPLIER)); - console2.log("fee recipient balance", vault.balanceOf(FEE_RECIPIENT)); + assertEq(vault.balanceOf(FEE_RECIPIENT), feeRecipientBalance, "vault.balanceOf(FEE_RECIPIENT)"); } function testShouldMintSharesToFeeRecipient(uint256 amount) public {