From 0f64b92f40ceea11f63f60a04766a4b8614ef5d4 Mon Sep 17 00:00:00 2001 From: Rubilmax Date: Fri, 22 Dec 2023 09:28:57 +0100 Subject: [PATCH 1/4] fix(metamorpho): accrue fee on reallocate --- src/MetaMorpho.sol | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/MetaMorpho.sol b/src/MetaMorpho.sol index c2749d59..a4769f77 100644 --- a/src/MetaMorpho.sol +++ b/src/MetaMorpho.sol @@ -229,7 +229,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph if (newFee > ConstantsLib.MAX_FEE) revert ErrorsLib.MaxFeeExceeded(); if (newFee != 0 && feeRecipient == address(0)) revert ErrorsLib.ZeroFeeRecipient(); - // Accrue interest using the previous fee set before changing it. + // Accrue fee using the previous fee set before changing it. _updateLastTotalAssets(_accrueFee()); // Safe "unchecked" cast because newFee <= MAX_FEE. @@ -243,7 +243,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph if (newFeeRecipient == feeRecipient) revert ErrorsLib.AlreadySet(); if (newFeeRecipient == address(0) && fee != 0) revert ErrorsLib.ZeroFeeRecipient(); - // Accrue interest to the previous fee recipient set before changing it. + // Accrue fee to the previous fee recipient set before changing it. _updateLastTotalAssets(_accrueFee()); feeRecipient = newFeeRecipient; @@ -365,6 +365,9 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph /// @inheritdoc IMetaMorphoBase function reallocate(MarketAllocation[] calldata allocations) external onlyAllocatorRole { + // Accrue fee to avoid taking a fee on withdrawn donations. + _accrueFee(); + uint256 totalSupplied; uint256 totalWithdrawn; for (uint256 i; i < allocations.length; ++i) { @@ -413,6 +416,9 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph } if (totalWithdrawn != totalSupplied) revert ErrorsLib.InconsistentReallocation(); + + // Update `lastTotalAssets` to avoid taking a fee on withdrawn donations. + _updateLastTotalAssets(totalAssets()); } /* REVOKE FUNCTIONS */ From aeb7ac1d7173d293151b51548585051fd64c0eab Mon Sep 17 00:00:00 2001 From: Rubilmax Date: Fri, 22 Dec 2023 15:11:14 +0100 Subject: [PATCH 2/4] fix(reallocate): prevent withdrawing from not enabled market --- src/MetaMorpho.sol | 10 ++-------- src/libraries/ErrorsLib.sol | 2 +- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/MetaMorpho.sol b/src/MetaMorpho.sol index a4769f77..82546577 100644 --- a/src/MetaMorpho.sol +++ b/src/MetaMorpho.sol @@ -294,7 +294,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph /// @inheritdoc IMetaMorphoBase function submitMarketRemoval(Id id) external onlyCuratorRole { if (config[id].removableAt != 0) revert ErrorsLib.AlreadySet(); - if (!config[id].enabled) revert ErrorsLib.MarketNotEnabled(); + if (!config[id].enabled) revert ErrorsLib.MarketNotEnabled(id); _setCap(id, 0); @@ -365,9 +365,6 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph /// @inheritdoc IMetaMorphoBase function reallocate(MarketAllocation[] calldata allocations) external onlyAllocatorRole { - // Accrue fee to avoid taking a fee on withdrawn donations. - _accrueFee(); - uint256 totalSupplied; uint256 totalWithdrawn; for (uint256 i; i < allocations.length; ++i) { @@ -378,7 +375,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph uint256 withdrawn = supplyAssets.zeroFloorSub(allocation.assets); if (withdrawn > 0) { - if (allocation.marketParams.loanToken != asset()) revert ErrorsLib.InconsistentAsset(id); + if (!config[id].enabled) revert ErrorsLib.MarketNotEnabled(id); // Guarantees that unknown frontrunning donations can be withdrawn, in order to disable a market. uint256 shares; @@ -416,9 +413,6 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph } if (totalWithdrawn != totalSupplied) revert ErrorsLib.InconsistentReallocation(); - - // Update `lastTotalAssets` to avoid taking a fee on withdrawn donations. - _updateLastTotalAssets(totalAssets()); } /* REVOKE FUNCTIONS */ diff --git a/src/libraries/ErrorsLib.sol b/src/libraries/ErrorsLib.sol index c5f3ed6d..6e731f86 100644 --- a/src/libraries/ErrorsLib.sol +++ b/src/libraries/ErrorsLib.sol @@ -64,7 +64,7 @@ library ErrorsLib { error MarketNotCreated(); /// @notice Thrown when submitting a non previously enabled market for removal. - error MarketNotEnabled(); + error MarketNotEnabled(Id id); /// @notice Thrown when the submitted timelock is above the max timelock. error AboveMaxTimelock(); From dd781a3ea9c60b0d336c8ceff28f260a0bfc3df3 Mon Sep 17 00:00:00 2001 From: Rubilmax Date: Fri, 22 Dec 2023 15:18:23 +0100 Subject: [PATCH 3/4] test(reallocate): add market not enabled --- test/forge/ReallocateWithdrawTest.sol | 4 ++-- test/forge/TimelockTest.sol | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/forge/ReallocateWithdrawTest.sol b/test/forge/ReallocateWithdrawTest.sol index a335e18e..5e0d0868 100644 --- a/test/forge/ReallocateWithdrawTest.sol +++ b/test/forge/ReallocateWithdrawTest.sol @@ -59,7 +59,7 @@ contract ReallocateWithdrawTest is IntegrationTest { assertEq(_idle(), INITIAL_DEPOSIT, "idle"); } - function testReallocateWithdrawInconsistentAsset() public { + function testReallocateWithdrawMarketNotEnabled() public { ERC20Mock loanToken2 = new ERC20Mock("loan2", "B2"); allMarkets[0].loanToken = address(loanToken2); @@ -75,7 +75,7 @@ contract ReallocateWithdrawTest is IntegrationTest { allocations.push(MarketAllocation(allMarkets[0], 0)); vm.prank(ALLOCATOR); - vm.expectRevert(abi.encodeWithSelector(ErrorsLib.InconsistentAsset.selector, allMarkets[0].id())); + vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MarketNotEnabled.selector, allMarkets[0].id())); vault.reallocate(allocations); } diff --git a/test/forge/TimelockTest.sol b/test/forge/TimelockTest.sol index 3ca97a1c..d8b3bc23 100644 --- a/test/forge/TimelockTest.sol +++ b/test/forge/TimelockTest.sol @@ -455,7 +455,7 @@ contract TimelockTest is IntegrationTest { } function testSubmitMarketRemovalMarketNotEnabled() public { - vm.expectRevert(ErrorsLib.MarketNotEnabled.selector); + vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MarketNotEnabled.selector, allMarkets[1].id())); vm.prank(CURATOR); vault.submitMarketRemoval(allMarkets[1].id()); } From 44fe10d13397d700fa419368e855d8aaba88124c Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Wed, 27 Dec 2023 13:49:20 +0100 Subject: [PATCH 4/4] docs: apply suggestion --- src/libraries/ErrorsLib.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/ErrorsLib.sol b/src/libraries/ErrorsLib.sol index 6e731f86..1e54c83a 100644 --- a/src/libraries/ErrorsLib.sol +++ b/src/libraries/ErrorsLib.sol @@ -63,7 +63,7 @@ library ErrorsLib { /// @notice Thrown when submitting a cap for a market which does not exist. error MarketNotCreated(); - /// @notice Thrown when submitting a non previously enabled market for removal. + /// @notice Thrown when interacting with a non previously enabled market `id`. error MarketNotEnabled(Id id); /// @notice Thrown when the submitted timelock is above the max timelock.