Merge pull request #237 from morpho-labs/fix/bound-supply-queue-size

fix: check supply queue size
morpho-org · Oct 20, 2023 · 2f8dec5 · 2f8dec5
2 parents 80b56e2 + 6e031a9
commit 2f8dec5
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 13 deletions.
diff --git a/src/MetaMorpho.sol b/src/MetaMorpho.sol
@@ -283,7 +283,11 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
     /// @dev The supply queue can be a set containing duplicate markets, but it would only increase the cost of
     /// depositing to the vault.
     function setSupplyQueue(Id[] calldata newSupplyQueue) external onlyAllocatorRole {
-        for (uint256 i; i < newSupplyQueue.length; ++i) {
+        uint256 length = newSupplyQueue.length;
+
+        if (length > ConstantsLib.MAX_QUEUE_SIZE) revert ErrorsLib.MaxQueueSizeExceeded();
+
+        for (uint256 i; i < length; ++i) {
             if (config[newSupplyQueue[i]].cap == 0) revert ErrorsLib.UnauthorizedMarket(newSupplyQueue[i]);
         }
 
@@ -666,7 +670,10 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
             supplyQueue.push(id);
             withdrawQueue.push(id);
 
-            if (withdrawQueue.length > ConstantsLib.MAX_QUEUE_SIZE) revert ErrorsLib.MaxQueueSizeExceeded();
+            if (supplyQueue.length > ConstantsLib.MAX_QUEUE_SIZE || withdrawQueue.length > ConstantsLib.MAX_QUEUE_SIZE)
+            {
+                revert ErrorsLib.MaxQueueSizeExceeded();
+            }
 
             // Safe "unchecked" cast because withdrawQueue.length <= MAX_QUEUE_SIZE.
             marketConfig.withdrawRank = uint64(withdrawQueue.length);

diff --git a/test/forge/MarketTest.sol b/test/forge/MarketTest.sol
@@ -65,6 +65,32 @@ contract MarketTest is IntegrationTest {
         assertEq(Id.unwrap(vault.supplyQueue(1)), Id.unwrap(allMarkets[2].id()));
     }
 
+    function testSetSupplyQueueMaxQueueSizeExceeded() public {
+        Id[] memory supplyQueue = new Id[](ConstantsLib.MAX_QUEUE_SIZE + 1);
+
+        vm.prank(ALLOCATOR);
+        vm.expectRevert(ErrorsLib.MaxQueueSizeExceeded.selector);
+        vault.setSupplyQueue(supplyQueue);
+    }
+
+    function testAcceptCapMaxQueueSizeExceeded() public {
+        for (uint256 i; i < ConstantsLib.MAX_QUEUE_SIZE; ++i) {
+            _setCap(allMarkets[i], CAP);
+        }
+
+        _setTimelock(1 weeks);
+
+        MarketParams memory marketParams = allMarkets[ConstantsLib.MAX_QUEUE_SIZE];
+
+        vm.prank(CURATOR);
+        vault.submitCap(marketParams, CAP);
+
+        vm.warp(block.timestamp + 1 weeks);
+
+        vm.expectRevert(ErrorsLib.MaxQueueSizeExceeded.selector);
+        vault.acceptCap(marketParams.id());
+    }
+
     function testSetSupplyQueueUnauthorizedMarket() public {
         Id[] memory supplyQueue = new Id[](1);
         supplyQueue[0] = allMarkets[0].id();

diff --git a/test/forge/MetaMorphoInternalTest.sol b/test/forge/MetaMorphoInternalTest.sol
@@ -14,10 +14,6 @@ contract MetaMorphoInternalTest is InternalTest {
     using SharesMathLib for uint256;
     using UtilsLib for uint256;
 
-    constructor() {
-        NB_MARKETS = ConstantsLib.MAX_QUEUE_SIZE + 1;
-    }
-
     function testSetCapMaxQueueSizeExcedeed() public {
         for (uint256 i; i < NB_MARKETS - 1; ++i) {
             Id id = allMarkets[i].id();

diff --git a/test/forge/helpers/BaseTest.sol b/test/forge/helpers/BaseTest.sol
@@ -30,15 +30,14 @@ uint256 constant BLOCK_TIME = 1;
 uint256 constant MIN_TEST_ASSETS = 1e8;
 uint256 constant MAX_TEST_ASSETS = 1e28;
 uint192 constant CAP = type(uint128).max;
+uint256 constant NB_MARKETS = ConstantsLib.MAX_QUEUE_SIZE + 1;
 
 contract BaseTest is Test {
     using MathLib for uint256;
     using MorphoLib for IMorpho;
     using MorphoBalancesLib for IMorpho;
     using MarketParamsLib for MarketParams;
 
-    uint256 internal immutable NB_MARKETS = 10;
-
     address internal OWNER = makeAddr("Owner");
     address internal SUPPLIER = makeAddr("Supplier");
     address internal BORROWER = makeAddr("Borrower");

diff --git a/test/metamorpho_tests.tree b/test/metamorpho_tests.tree
@@ -199,6 +199,8 @@
                 ├── when supplyCap > 0 and marketConfig.withdrawRank == 0
                 │   ├── it should push id to supplyQueue
                 │   ├── it should push id to withdrawQueue
+                │   ├── if supplyQueue.length > MAX_QUEUE_SIZE
+                │   │   └── revert with MaxQueueSizeExceeded()
                 │   └── if withdrawQueue.length > MAX_QUEUE_SIZE
                 │       └── revert with MaxQueueSizeExceeded()
                 ├── it should set config[id].cap to pendingCap[id].value
@@ -214,11 +216,14 @@
     ├── when msg.sender not owner or curator or allocator
     │   └── revert with NotAllocatorRole()
     └── when msg.sender is owner or curator or allocator
-        ├── when some markets of newSupplyQueue have a zero cap
-        │   └── revert with UnauthorizedMarket()
-        └── when all the markets of newSupplyQueue have non zero cap
-            ├── it should set supplyQueue to newSupplyQueue
-            └── it shoud emit SetSupplyQueue(msg.sender, newSupplyQueue)
+        ├── when newSupplyQueue.length > MAX_QUEUE_SIZE
+        │   └── revert with MaxQueueSizeExceeded()
+        └── when newSupplyQueue.length <= MAX_QUEUE_SIZE
+            ├── when some markets of newSupplyQueue have a zero cap
+            │   └── revert with UnauthorizedMarket()
+            └── when all the markets of newSupplyQueue have non zero cap
+                ├── it should set supplyQueue to newSupplyQueue
+                └── it shoud emit SetSupplyQueue(msg.sender, newSupplyQueue)
 
 .
 └── sortWithdrawQueue(uint256[] calldata indexes) external