From cdfd66452f450f2dcf937659b68a8acd78b11a70 Mon Sep 17 00:00:00 2001 From: Jean-Grimal Date: Thu, 19 Oct 2023 17:42:15 +0200 Subject: [PATCH 1/3] fix: check supply queue size --- src/MetaMorpho.sol | 2 ++ test/forge/MarketTest.sol | 8 ++++++++ test/metamorpho_tests.tree | 13 ++++++++----- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/MetaMorpho.sol b/src/MetaMorpho.sol index b64f6656..a52e7634 100644 --- a/src/MetaMorpho.sol +++ b/src/MetaMorpho.sol @@ -272,6 +272,8 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph function setSupplyQueue(Id[] calldata newSupplyQueue) external onlyAllocator { uint256 length = newSupplyQueue.length; + if (length > MAX_QUEUE_SIZE) revert ErrorsLib.MaxQueueSizeExceeded(); + for (uint256 i; i < length; ++i) { if (config[newSupplyQueue[i]].cap == 0) revert ErrorsLib.UnauthorizedMarket(newSupplyQueue[i]); } diff --git a/test/forge/MarketTest.sol b/test/forge/MarketTest.sol index 72ac3f4a..3f3690b7 100644 --- a/test/forge/MarketTest.sol +++ b/test/forge/MarketTest.sol @@ -65,6 +65,14 @@ contract MarketTest is IntegrationTest { assertEq(Id.unwrap(vault.supplyQueue(1)), Id.unwrap(allMarkets[2].id())); } + function testSetSupplyQueueMaxQueueSizeExceeded() public { + Id[] memory supplyQueue = new Id[](MAX_QUEUE_SIZE + 1); + + vm.prank(ALLOCATOR); + vm.expectRevert(ErrorsLib.MaxQueueSizeExceeded.selector); + vault.setSupplyQueue(supplyQueue); + } + function testSetSupplyQueueUnauthorizedMarket() public { Id[] memory supplyQueue = new Id[](1); supplyQueue[0] = allMarkets[0].id(); diff --git a/test/metamorpho_tests.tree b/test/metamorpho_tests.tree index 7b98a9d2..30ef1755 100644 --- a/test/metamorpho_tests.tree +++ b/test/metamorpho_tests.tree @@ -214,11 +214,14 @@ ├── when msg.sender not owner or curator or allocator │ └── revert with NotAllocator() └── 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 From c6772815b3fecbf05b2e8e301f1b34f1c1926836 Mon Sep 17 00:00:00 2001 From: Rubilmax Date: Fri, 20 Oct 2023 18:51:12 +0200 Subject: [PATCH 2/3] fix(accept-cap): revert when max queue size exceeded --- src/MetaMorpho.sol | 5 ++++- test/forge/MarketTest.sol | 18 ++++++++++++++++++ test/forge/MetaMorphoInternalTest.sol | 4 ---- test/forge/helpers/BaseTest.sol | 3 +-- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/MetaMorpho.sol b/src/MetaMorpho.sol index aea06da1..93d6052e 100644 --- a/src/MetaMorpho.sol +++ b/src/MetaMorpho.sol @@ -661,7 +661,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 index 6c2a967b..c2aa0e85 100644 --- a/test/forge/MarketTest.sol +++ b/test/forge/MarketTest.sol @@ -73,6 +73,24 @@ contract MarketTest is IntegrationTest { 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 index dde14a3f..ba6338da 100644 --- 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 index 283e0fa1..c337aab0 100644 --- a/test/forge/helpers/BaseTest.sol +++ b/test/forge/helpers/BaseTest.sol @@ -30,6 +30,7 @@ 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; @@ -37,8 +38,6 @@ contract BaseTest is Test { 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"); From 1d1e65f2f8127175b33971fbfd7ca8abaedfa42e Mon Sep 17 00:00:00 2001 From: Rubilmax Date: Fri, 20 Oct 2023 18:52:12 +0200 Subject: [PATCH 3/3] refactor(btt): add edge-case to btt --- test/metamorpho_tests.tree | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/metamorpho_tests.tree b/test/metamorpho_tests.tree index 311da7d0..f45af68f 100644 --- 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