From 6335a154863c0a7af185905daa65751e1336cba2 Mon Sep 17 00:00:00 2001 From: Adrien Husson Date: Fri, 16 Feb 2024 21:04:50 +0100 Subject: [PATCH 1/3] feat: custom overflow errors --- src/PublicAllocator.sol | 3 +++ src/libraries/ErrorsLib.sol | 9 +++++++ test/PublicAllocatorTest.sol | 49 ++++++++++++++++++++++++++++++++++-- 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/PublicAllocator.sol b/src/PublicAllocator.sol index 0ef9504..10cb299 100644 --- a/src/PublicAllocator.sol +++ b/src/PublicAllocator.sol @@ -103,7 +103,9 @@ contract PublicAllocator is IPublicAllocatorStaticTyping { uint128 withdrawnAssets = withdrawals[i].amount; totalWithdrawn += withdrawnAssets; flowCap[id].maxIn += withdrawnAssets; + if (flowCap[id].maxOut < withdrawnAssets) revert ErrorsLib.MaxOutflowExceeded(id); flowCap[id].maxOut -= withdrawnAssets; + if (assets < withdrawnAssets) revert ErrorsLib.NotEnoughSupply(id); allocations[i].assets = assets - withdrawnAssets; allocations[i].marketParams = withdrawals[i].marketParams; @@ -112,6 +114,7 @@ contract PublicAllocator is IPublicAllocatorStaticTyping { allocations[withdrawals.length].marketParams = supplyMarketParams; allocations[withdrawals.length].assets = type(uint256).max; + if (flowCap[supplyMarketId].maxIn < totalWithdrawn) revert ErrorsLib.MaxInflowExceeded(supplyMarketId); flowCap[supplyMarketId].maxIn -= totalWithdrawn; flowCap[supplyMarketId].maxOut += totalWithdrawn; diff --git a/src/libraries/ErrorsLib.sol b/src/libraries/ErrorsLib.sol index 59c9320..274a0e1 100644 --- a/src/libraries/ErrorsLib.sol +++ b/src/libraries/ErrorsLib.sol @@ -31,4 +31,13 @@ library ErrorsLib { /// @notice Thrown when the PublicAllocatorFactory is called with a vault not made by the MetaMorphoFactory. error NotMetaMorpho(); + + /// @notice Thrown when attempting to withdraw more than the available supply of a market. + error NotEnoughSupply(Id id); + + /// @notice Thrown when attempting to withdraw more than the max outflow of a market. + error MaxOutflowExceeded(Id id); + + /// @notice Thrown when attempting to supply more than the max inflow of a market. + error MaxInflowExceeded(Id id); } diff --git a/test/PublicAllocatorTest.sol b/test/PublicAllocatorTest.sol index 0ebd0ee..4f23a64 100644 --- a/test/PublicAllocatorTest.sol +++ b/test/PublicAllocatorTest.sol @@ -77,7 +77,7 @@ contract PublicAllocatorTest is IntegrationTest { vm.prank(OWNER); publicAllocator.setFlowCaps(flowCaps); withdrawals.push(Withdrawal(idleParams, flow)); - vm.expectRevert(stdError.arithmeticError); + vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MaxOutflowExceeded.selector,idleParams.id())); publicAllocator.reallocateTo(withdrawals, allMarkets[0]); } @@ -88,7 +88,7 @@ contract PublicAllocatorTest is IntegrationTest { vm.prank(OWNER); publicAllocator.setFlowCaps(flowCaps); withdrawals.push(Withdrawal(idleParams, flow)); - vm.expectRevert(stdError.arithmeticError); + vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MaxInflowExceeded.selector,allMarkets[0].id())); publicAllocator.reallocateTo(withdrawals, allMarkets[0]); } @@ -484,4 +484,49 @@ contract PublicAllocatorTest is IntegrationTest { vm.prank(OWNER); publicAllocator.setFlowCaps(flowCaps); } + + function testNotEnoughSupply() public { + uint128 flow = 1e18; + // Set flow limits with withdraw market's maxIn to max + 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))); + vm.prank(OWNER); + publicAllocator.setFlowCaps(flowCaps); + + withdrawals.push(Withdrawal(idleParams, flow)); + publicAllocator.reallocateTo(withdrawals, allMarkets[0]); + + delete withdrawals; + + withdrawals.push(Withdrawal(allMarkets[0],flow+1)); + vm.expectRevert(abi.encodeWithSelector(ErrorsLib.NotEnoughSupply.selector,allMarkets[0].id())); + publicAllocator.reallocateTo(withdrawals,idleParams); + } + + function testMaxOutflowExceeded() public { + uint128 cap = 1e18; + // Set flow limits with withdraw market's maxIn to max + flowCaps.push(FlowConfig(idleParams.id(), FlowCap(MAX_SETTABLE_FLOW_CAP, cap))); + flowCaps.push(FlowConfig(allMarkets[0].id(), FlowCap(MAX_SETTABLE_FLOW_CAP, MAX_SETTABLE_FLOW_CAP))); + vm.prank(OWNER); + publicAllocator.setFlowCaps(flowCaps); + + withdrawals.push(Withdrawal(idleParams, cap+1)); + vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MaxOutflowExceeded.selector,idleParams.id())); + publicAllocator.reallocateTo(withdrawals, allMarkets[0]); + } + + + function testMaxInflowExceeded() public { + uint128 cap = 1e18; + // Set flow limits with withdraw market's maxIn to max + flowCaps.push(FlowConfig(idleParams.id(), FlowCap(MAX_SETTABLE_FLOW_CAP, MAX_SETTABLE_FLOW_CAP))); + flowCaps.push(FlowConfig(allMarkets[0].id(), FlowCap(cap, MAX_SETTABLE_FLOW_CAP))); + vm.prank(OWNER); + publicAllocator.setFlowCaps(flowCaps); + + withdrawals.push(Withdrawal(idleParams, cap+1)); + vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MaxInflowExceeded.selector,allMarkets[0].id())); + publicAllocator.reallocateTo(withdrawals, allMarkets[0]); + } } From 9d112659501030dfb72ada65e61b5eb0e1f753c8 Mon Sep 17 00:00:00 2001 From: MathisGD Date: Sat, 17 Feb 2024 12:48:32 +0100 Subject: [PATCH 2/3] refactor: minor change in order --- src/PublicAllocator.sol | 15 +++++++++------ test/PublicAllocatorFactoryTest.sol | 8 ++------ test/PublicAllocatorTest.sol | 19 +++++++++---------- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/PublicAllocator.sol b/src/PublicAllocator.sol index 10cb299..2797f01 100644 --- a/src/PublicAllocator.sol +++ b/src/PublicAllocator.sol @@ -99,24 +99,27 @@ contract PublicAllocator is IPublicAllocatorStaticTyping { MORPHO.accrueInterest(withdrawals[i].marketParams); uint256 assets = MORPHO.expectedSupplyAssets(withdrawals[i].marketParams, address(VAULT)); - uint128 withdrawnAssets = withdrawals[i].amount; - totalWithdrawn += withdrawnAssets; - flowCap[id].maxIn += withdrawnAssets; + if (flowCap[id].maxOut < withdrawnAssets) revert ErrorsLib.MaxOutflowExceeded(id); - flowCap[id].maxOut -= withdrawnAssets; if (assets < withdrawnAssets) revert ErrorsLib.NotEnoughSupply(id); + + flowCap[id].maxIn += withdrawnAssets; + flowCap[id].maxOut -= withdrawnAssets; allocations[i].assets = assets - withdrawnAssets; allocations[i].marketParams = withdrawals[i].marketParams; + totalWithdrawn += withdrawnAssets; + emit EventsLib.PublicWithdrawal(id, withdrawnAssets); } - allocations[withdrawals.length].marketParams = supplyMarketParams; - allocations[withdrawals.length].assets = type(uint256).max; if (flowCap[supplyMarketId].maxIn < totalWithdrawn) revert ErrorsLib.MaxInflowExceeded(supplyMarketId); + flowCap[supplyMarketId].maxIn -= totalWithdrawn; flowCap[supplyMarketId].maxOut += totalWithdrawn; + allocations[withdrawals.length].marketParams = supplyMarketParams; + allocations[withdrawals.length].assets = type(uint256).max; VAULT.reallocate(allocations); 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 4f23a64..5a04258 100644 --- a/test/PublicAllocatorTest.sol +++ b/test/PublicAllocatorTest.sol @@ -77,7 +77,7 @@ contract PublicAllocatorTest is IntegrationTest { vm.prank(OWNER); publicAllocator.setFlowCaps(flowCaps); withdrawals.push(Withdrawal(idleParams, flow)); - vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MaxOutflowExceeded.selector,idleParams.id())); + vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MaxOutflowExceeded.selector, idleParams.id())); publicAllocator.reallocateTo(withdrawals, allMarkets[0]); } @@ -88,7 +88,7 @@ contract PublicAllocatorTest is IntegrationTest { vm.prank(OWNER); publicAllocator.setFlowCaps(flowCaps); withdrawals.push(Withdrawal(idleParams, flow)); - vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MaxInflowExceeded.selector,allMarkets[0].id())); + vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MaxInflowExceeded.selector, allMarkets[0].id())); publicAllocator.reallocateTo(withdrawals, allMarkets[0]); } @@ -498,9 +498,9 @@ contract PublicAllocatorTest is IntegrationTest { delete withdrawals; - withdrawals.push(Withdrawal(allMarkets[0],flow+1)); - vm.expectRevert(abi.encodeWithSelector(ErrorsLib.NotEnoughSupply.selector,allMarkets[0].id())); - publicAllocator.reallocateTo(withdrawals,idleParams); + withdrawals.push(Withdrawal(allMarkets[0], flow + 1)); + vm.expectRevert(abi.encodeWithSelector(ErrorsLib.NotEnoughSupply.selector, allMarkets[0].id())); + publicAllocator.reallocateTo(withdrawals, idleParams); } function testMaxOutflowExceeded() public { @@ -511,12 +511,11 @@ contract PublicAllocatorTest is IntegrationTest { vm.prank(OWNER); publicAllocator.setFlowCaps(flowCaps); - withdrawals.push(Withdrawal(idleParams, cap+1)); - vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MaxOutflowExceeded.selector,idleParams.id())); + withdrawals.push(Withdrawal(idleParams, cap + 1)); + vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MaxOutflowExceeded.selector, idleParams.id())); publicAllocator.reallocateTo(withdrawals, allMarkets[0]); } - function testMaxInflowExceeded() public { uint128 cap = 1e18; // Set flow limits with withdraw market's maxIn to max @@ -525,8 +524,8 @@ contract PublicAllocatorTest is IntegrationTest { vm.prank(OWNER); publicAllocator.setFlowCaps(flowCaps); - withdrawals.push(Withdrawal(idleParams, cap+1)); - vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MaxInflowExceeded.selector,allMarkets[0].id())); + withdrawals.push(Withdrawal(idleParams, cap + 1)); + vm.expectRevert(abi.encodeWithSelector(ErrorsLib.MaxInflowExceeded.selector, allMarkets[0].id())); publicAllocator.reallocateTo(withdrawals, allMarkets[0]); } } From d0fb924ed4b8a200654a8b041180cbe3f555b3c7 Mon Sep 17 00:00:00 2001 From: MathisGD Date: Sat, 17 Feb 2024 12:54:01 +0100 Subject: [PATCH 3/3] refactor: minor change in order --- src/PublicAllocator.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PublicAllocator.sol b/src/PublicAllocator.sol index 6b2724e..3d83fd8 100644 --- a/src/PublicAllocator.sol +++ b/src/PublicAllocator.sol @@ -103,8 +103,8 @@ contract PublicAllocator is IPublicAllocatorStaticTyping { flowCap[id].maxIn += withdrawnAssets; flowCap[id].maxOut -= withdrawnAssets; - allocations[i].assets = assets - withdrawnAssets; allocations[i].marketParams = withdrawals[i].marketParams; + allocations[i].assets = assets - withdrawnAssets; totalWithdrawn += withdrawnAssets;