diff --git a/README.md b/README.md index 3ee93931..8c5794c0 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,9 @@ The [`MetaMorphoFactory`](./src/MetaMorphoFactory.sol) is deploying immutable on Users can supply or withdraw assets at any time, depending on the available liquidity on Morpho Blue. A maximum of 30 markets can be enabled on a given MetaMorpho vault. -Each market has a supply cap that guarantees lenders a maximum absolute exposure to the specific market. By default, the supply cap of a market is set to 0. +Each market has a supply cap that guarantees that allocators cannot reallocate more than that limit to that market. +The vault can have more exposure to a given market than its supply cap because of donations or because of interests. +By default, the supply cap of a market is set to 0. There are 4 different roles for a MetaMorpho vault: owner, curator, guardian & allocator. @@ -150,9 +152,11 @@ If an enabled market is considered unsafe (e.g., risk too high), the curator/own If there is not enough liquidity on the market, remove the maximum available liquidity with the `reallocate` function, then put the market at the beginning of the withdraw queue (with the `updateWithdrawQueue` function). - 4. Once all the supply has been removed from the market, the market can be removed from the withdraw queue with the `updateWithdrawQueue` function. -### An enabled market reverts +### Market funds are lost -If an enabled market starts reverting, many of the vault functions would revert as well (because of the call to `totalAssets`). To turn the vault back to an operating state, the market must be forced removed by the owner/curator, who should follow these steps: +To write off the funds in a faulty market, the curator can force remove that market. +If an enabled market starts reverting notably, many of the vault functions would revert as well (because of the call to `totalAssets`). +To turn the vault back to an operating state, the market must be forced removed by the owner/curator, who should follow these steps: - 1. Revoke the pending cap of the market with the `revokePendingCap` function (this can also be done by the guardian). - 2. Set the cap of the market to 0 with the `submitCap` function. @@ -161,7 +165,7 @@ If an enabled market starts reverting, many of the vault functions would revert - 4. Wait for the timelock to elapse - 5. Once the timelock has elapsed, the market can be removed from the withdraw queue with the `updateWithdrawQueue` function. -Warning : Funds supplied in forced removed markets will be lost, this is why only markets expected to always revert should be disabled this way (because funds supplied in such markets can be considered lost anyway). +Warning : Funds previously supplied in forced removed markets will be lost permanently. ### Curator takeover diff --git a/src/libraries/PendingLib.sol b/src/libraries/PendingLib.sol index 63741cfd..45be617b 100644 --- a/src/libraries/PendingLib.sol +++ b/src/libraries/PendingLib.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.0; struct MarketConfig { /// @notice The maximum amount of assets that can be allocated to the market. + /// @dev The exposure to a given market can go beyond the cap in case of interest or donations. uint184 cap; /// @notice Whether the market is in the withdraw queue. bool enabled; diff --git a/test/forge/ReentrancyTest.sol b/test/forge/ReentrancyTest.sol index 163278d5..89eb5126 100644 --- a/test/forge/ReentrancyTest.sol +++ b/test/forge/ReentrancyTest.sol @@ -84,21 +84,21 @@ contract ReentrancyTest is IntegrationTest, IERC1820Implementer { vm.startPrank(attacker); registry.setInterfaceImplementer(attacker, TOKENS_SENDER_INTERFACE_HASH, address(this)); // Set test contract - // to receive ERC-777 callbacks. + // to receive ERC-777 callbacks. registry.setInterfaceImplementer(attacker, TOKENS_RECIPIENT_INTERFACE_HASH, address(this)); // Required "hack" - // because done all in a single Foundry test. + // because done all in a single Foundry test. reentrantToken.approve(address(vault), 100_000); vault.deposit(1, attacker); // Initial deposit of 1 token to be able to call withdraw(1) in the subcall - // before depositing(5000) + // before depositing(5000) vault.deposit(5_000, attacker); // Deposit 5000, withdraw 1 in the subcall. Total deposited 4999, - // lastTotalAssets only updated by +1. + // lastTotalAssets only updated by +1. vm.startPrank(attacker); // Have to re-call startPrank because contract was reentered. Hacky but works. vault.deposit(5_000, attacker); // Same as above. Accrue yield accrues 50% * (newTotalAssets - - // lastTotalAssets) = 50% * 4999 = ~2499. lastTotalAssets only updated by +1. + // lastTotalAssets) = 50% * 4999 = ~2499. lastTotalAssets only updated by +1. vm.startPrank(attacker); vault.deposit(5_000, attacker); // ~2499 tokens taken as fees. @@ -110,11 +110,11 @@ contract ReentrancyTest is IntegrationTest, IERC1820Implementer { vm.startPrank(attacker); vault.withdraw(vault.maxWithdraw(attacker), attacker, attacker); // Withdraw 99_999 tokens, cost of attack - // = 1 token + // = 1 token vm.startPrank(FEE_RECIPIENT); vault.withdraw(vault.maxWithdraw(FEE_RECIPIENT), FEE_RECIPIENT, FEE_RECIPIENT); // Fee recipient withdraws - // 9_999 tokens, stolen from `SUPPLIER` + // 9_999 tokens, stolen from `SUPPLIER` console2.log("Attacker ending with %s tokens", reentrantToken.balanceOf(attacker)); // 99_999 console2.log("Fee recipient ending with %s tokens", reentrantToken.balanceOf(FEE_RECIPIENT)); // 9_999