sorted withdrawals

src/PublicAllocator.sol

@@ -86,16 +86,13 @@ contract PublicAllocator is IPublicAllocatorStaticTyping {
         Id supplyMarketId = supplyMarketParams.id();
         uint128 totalWithdrawn;
 
+        Id id;
+        Id prevId;
         for (uint256 i = 0; i < withdrawals.length; i++) {
-            Id id = withdrawals[i].marketParams.id();
-
-            // Revert if the market is elsewhere in the list, or is the supply market.
-            for (uint256 j = i + 1; j < withdrawals.length; j++) {
-                if (Id.unwrap(id) == Id.unwrap(withdrawals[j].marketParams.id())) {
-                    revert ErrorsLib.InconsistentWithdrawTo();
-                }
-            }
-            if (Id.unwrap(id) == Id.unwrap(supplyMarketId)) revert ErrorsLib.InconsistentWithdrawTo();
+            prevId = id;
+            id = withdrawals[i].marketParams.id();
+            if (Id.unwrap(id) <= Id.unwrap(prevId)) revert ErrorsLib.InconsistentWithdrawals();
+            if (Id.unwrap(id) == Id.unwrap(supplyMarketId)) revert ErrorsLib.DepositMarketInWithdrawals();
 
             MORPHO.accrueInterest(withdrawals[i].marketParams);
             uint256 assets = MORPHO.expectedSupplyAssets(withdrawals[i].marketParams, address(VAULT));

src/interfaces/IPublicAllocator.sol

@@ -64,6 +64,8 @@ interface IPublicAllocatorBase {
     /// @param supplyMarketParams The market receiving total withdrawn to.
     /// @dev Will call MetaMorpho's `reallocate`.
     /// @dev Checks that the public allocator constraints (flows, caps) are respected.
+    /// @dev Will revert when `withdrawals` contains a duplicate or is not sorted.
+    /// @dev Will revert if `withdrawals` contains the supply market.
     /// @dev Will revert if a withdrawal amount is larger than available liquidity.
     function reallocateTo(Withdrawal[] calldata withdrawals, MarketParams calldata supplyMarketParams)
         external

src/libraries/ErrorsLib.sol

@@ -23,8 +23,11 @@ library ErrorsLib {
     /// @notice Thrown when the value is already set.
     error AlreadySet();
 
-    /// @notice Thrown when there are duplicates with nonzero assets in `reallocateTo` arguments.
-    error InconsistentWithdrawTo();
+    /// @notice Thrown when `withdrawals` contains a duplicate or is not sorted.
+    error InconsistentWithdrawals();
+
+    /// @notice Thrown when the deposit market is in `withdrawals`.
+    error DepositMarketInWithdrawals();
 
     /// @notice Thrown when attempting to set max inflow/outflow above the MAX_SETTABLE_FLOW_CAP.
     error MaxSettableFlowCapExceeded();

test/PublicAllocatorFactoryTest.sol

@@ -25,19 +25,15 @@ contract PublicAllocatorFactoryTest is IntegrationTest {
         new PublicAllocatorFactory(address(0));
     }
 
-    function testCreatePublicAllocator(address initialOwner, address vault, bytes32 salt)
-        public
-    {
+    function testCreatePublicAllocator(address initialOwner, address vault, bytes32 salt) public {
         vm.assume(address(initialOwner) != address(0));
         vm.assume(address(vault) != address(0));
 
         bytes32 initCodeHash = hashInitCode(type(PublicAllocator).creationCode, abi.encode(initialOwner, vault));
         address expectedAddress = computeCreate2Address(salt, initCodeHash, address(factory));
 
         vm.mockCall(
-            metaMorphoFactory,
-            abi.encodeWithSelector(IMetaMorphoFactory.isMetaMorpho.selector, vault),
-            abi.encode(true)
+            metaMorphoFactory, abi.encodeWithSelector(IMetaMorphoFactory.isMetaMorpho.selector, vault), abi.encode(true)
         );
 
         vm.mockCall(vault, abi.encodeWithSelector(IMetaMorphoBase.MORPHO.selector), abi.encode(morpho));

test/PublicAllocatorTest.sol

@@ -28,12 +28,34 @@ contract CantReceive {
     }
 }
 
+// Withdrawal sorting snippet
+library SortWithdrawals {
+    using MarketParamsLib for MarketParams;
+    // Sorts withdrawals in-place using gnome sort.
+    // Does not detect duplicates.
+    // The sort will not be in-place if you pass a storage array.
+
+    function sort(Withdrawal[] memory ws) internal pure returns (Withdrawal[] memory) {
+        uint256 i;
+        while (i < ws.length) {
+            if (i == 0 || Id.unwrap(ws[i].marketParams.id()) >= Id.unwrap(ws[i - 1].marketParams.id())) {
+                i++;
+            } else {
+                (ws[i], ws[i - 1]) = (ws[i - 1], ws[i]);
+                i--;
+            }
+        }
+        return ws;
+    }
+}
+
 contract PublicAllocatorTest is IntegrationTest {
     IPublicAllocator public publicAllocator;
     Withdrawal[] internal withdrawals;
     FlowConfig[] internal flowCaps;
     SupplyConfig[] internal supplyCaps;
 
+    using SortWithdrawals for Withdrawal[];
     using MarketParamsLib for MarketParams;
     using MorphoBalancesLib for IMorpho;
 
@@ -212,7 +234,7 @@ contract PublicAllocatorTest is IntegrationTest {
         emit EventsLib.PublicReallocateTo(sender, allMarkets[0].id(), 2 * flow);
 
         vm.prank(sender);
-        publicAllocator.reallocateTo(withdrawals, allMarkets[0]);
+        publicAllocator.reallocateTo(withdrawals.sort(), allMarkets[0]);
     }
 
     function testReallocateNetting(uint128 flow) public {
@@ -448,7 +470,7 @@ contract PublicAllocatorTest is IntegrationTest {
         // _setCap(allMarkets[1], CAP2);
         withdrawals.push(Withdrawal(idleParams, 1e18));
         withdrawals.push(Withdrawal(idleParams, 1e18));
-        vm.expectRevert(ErrorsLib.InconsistentWithdrawTo.selector);
+        vm.expectRevert(ErrorsLib.InconsistentWithdrawals.selector);
         publicAllocator.reallocateTo(withdrawals, allMarkets[0]);
     }
 
@@ -460,7 +482,7 @@ contract PublicAllocatorTest is IntegrationTest {
         publicAllocator.setFlowCaps(flowCaps);
 
         withdrawals.push(Withdrawal(idleParams, 1e18));
-        vm.expectRevert(ErrorsLib.InconsistentWithdrawTo.selector);
+        vm.expectRevert(ErrorsLib.DepositMarketInWithdrawals.selector);
         publicAllocator.reallocateTo(withdrawals, idleParams);
     }
 
@@ -484,4 +506,32 @@ contract PublicAllocatorTest is IntegrationTest {
         vm.prank(OWNER);
         publicAllocator.setFlowCaps(flowCaps);
     }
+
+    function testReallocateToNotSorted() public {
+        // Prepare public reallocation from 2 markets to 1
+        _setCap(allMarkets[1], CAP2);
+
+        MarketAllocation[] memory allocations = new MarketAllocation[](3);
+        allocations[0] = MarketAllocation(idleParams, INITIAL_DEPOSIT - 2e18);
+        allocations[1] = MarketAllocation(allMarkets[0], 1e18);
+        allocations[2] = MarketAllocation(allMarkets[1], 1e18);
+        vm.prank(OWNER);
+        vault.reallocate(allocations);
+
+        flowCaps.push(FlowConfig(idleParams.id(), FlowCap(MAX_SETTABLE_FLOW_CAP, MAX_SETTABLE_FLOW_CAP)));
+        flowCaps.push(FlowConfig(allMarkets[0].id(), FlowCap(MAX_SETTABLE_FLOW_CAP, MAX_SETTABLE_FLOW_CAP)));
+        flowCaps.push(FlowConfig(allMarkets[1].id(), FlowCap(MAX_SETTABLE_FLOW_CAP, MAX_SETTABLE_FLOW_CAP)));
+        vm.prank(OWNER);
+        publicAllocator.setFlowCaps(flowCaps);
+
+        withdrawals.push(Withdrawal(allMarkets[0], 1e18));
+        withdrawals.push(Withdrawal(allMarkets[1], 1e18));
+        Withdrawal[] memory sortedWithdrawals = withdrawals.sort();
+        // Created non-sorted withdrawals list
+        withdrawals[0] = sortedWithdrawals[1];
+        withdrawals[1] = sortedWithdrawals[0];
+
+        vm.expectRevert(ErrorsLib.InconsistentWithdrawals.selector);
+        publicAllocator.reallocateTo(withdrawals, idleParams);
+    }
 }