diff --git a/src/PublicAllocator.sol b/src/PublicAllocator.sol index 0ef9504..4bed5c7 100644 --- a/src/PublicAllocator.sol +++ b/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)); diff --git a/src/interfaces/IPublicAllocator.sol b/src/interfaces/IPublicAllocator.sol index 3fb36ae..5b731c8 100644 --- a/src/interfaces/IPublicAllocator.sol +++ b/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 diff --git a/src/libraries/ErrorsLib.sol b/src/libraries/ErrorsLib.sol index 59c9320..563bcac 100644 --- a/src/libraries/ErrorsLib.sol +++ b/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(); diff --git a/test/PublicAllocatorFactoryTest.sol b/test/PublicAllocatorFactoryTest.sol index 41c6148..3a9d67e 100644 --- a/test/PublicAllocatorFactoryTest.sol +++ b/test/PublicAllocatorFactoryTest.sol @@ -25,9 +25,7 @@ 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)); @@ -35,9 +33,7 @@ contract PublicAllocatorFactoryTest is IntegrationTest { 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)); diff --git a/test/PublicAllocatorTest.sol b/test/PublicAllocatorTest.sol index 0ebd0ee..08fcaa5 100644 --- a/test/PublicAllocatorTest.sol +++ b/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); + } }