posm: CLEAR_OR_TAKE (#252)

* clear or take

* misc code comment

* update naming; use delta resolver helper for clearOrTake

* nits

* reorder

* nits

.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap

@@ -1 +1 @@
-140844
+140956

.forge-snapshots/V4Router_Bytecode.snap

@@ -1 +1 @@
-8068
+8116

src/PositionManager.sol

@@ -48,6 +48,7 @@ contract PositionManager is
     using StateLibrary for IPoolManager;
     using TransientStateLibrary for IPoolManager;
     using SafeCast for uint256;
+    using SafeCast for int256;
     using CalldataDecoder for bytes;
     using SlippageCheckLibrary for BalanceDelta;
 
@@ -155,9 +156,9 @@ contract PositionManager is
         } else if (action == Actions.CLOSE_CURRENCY) {
             Currency currency = params.decodeCurrency();
             _close(currency);
-        } else if (action == Actions.CLEAR) {
+        } else if (action == Actions.CLEAR_OR_TAKE) {
             (Currency currency, uint256 amountMax) = params.decodeCurrencyAndUint256();
-            _clear(currency, amountMax);
+            _clearOrTake(currency, amountMax);
         } else if (action == Actions.BURN_POSITION) {
             // Will automatically decrease liquidity to 0 if the position is not already empty.
             (
@@ -251,18 +252,23 @@ contract PositionManager is
     }
 
     /// @dev integrators may elect to forfeit positive deltas with clear
-    /// provides a safety check that amount-to-clear is less than a user-provided maximum
-    function _clear(Currency currency, uint256 amountMax) internal {
-        int256 currencyDelta = poolManager.currencyDelta(address(this), currency);
-        if (uint256(currencyDelta) > amountMax) revert ClearExceedsMaxAmount(currency, currencyDelta, amountMax);
-        poolManager.clear(currency, uint256(currencyDelta));
+    /// if the forfeit amount exceeds the user-specified max, the amount is taken instead
+    function _clearOrTake(Currency currency, uint256 amountMax) internal {
+        uint256 delta = _getFullCredit(currency);
+
+        // forfeit the delta if its less than or equal to the user-specified limit
+        if (delta <= amountMax) {
+            poolManager.clear(currency, delta);
+        } else {
+            _take(currency, msgSender(), delta);
+        }
     }
 
     function _settlePair(Currency currency0, Currency currency1) internal {
         // the locker is the payer when settling
         address caller = msgSender();
-        _settle(currency0, caller, _getFullSettleAmount(currency0));
-        _settle(currency1, caller, _getFullSettleAmount(currency1));
+        _settle(currency0, caller, _getFullDebt(currency0));
+        _settle(currency1, caller, _getFullDebt(currency1));
     }
 
     /// @dev this is overloaded with ERC721Permit._burn

src/V4Router.sol

@@ -53,17 +53,17 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver {
             if (action == Actions.SETTLE_ALL) {
                 Currency currency = params.decodeCurrency();
                 // TODO should it have a maxAmountOut added slippage protection?
-                _settle(currency, msgSender(), _getFullSettleAmount(currency));
+                _settle(currency, msgSender(), _getFullDebt(currency));
             } else if (action == Actions.SETTLE) {
                 (Currency currency, uint256 amount, bool payerIsUser) = params.decodeCurrencyUint256AndBool();
                 _settle(currency, _mapPayer(payerIsUser), _mapSettleAmount(amount, currency));
             } else if (action == Actions.TAKE_ALL) {
                 (Currency currency, address recipient) = params.decodeCurrencyAndAddress();
-                uint256 amount = _getFullTakeAmount(currency);
+                uint256 amount = _getFullCredit(currency);
                 _take(currency, _mapRecipient(recipient), amount);
             } else if (action == Actions.TAKE_PORTION) {
                 (Currency currency, address recipient, uint256 bips) = params.decodeCurrencyAddressAndUint256();
-                _take(currency, _mapRecipient(recipient), _getFullTakeAmount(currency).calculatePortion(bips));
+                _take(currency, _mapRecipient(recipient), _getFullCredit(currency).calculatePortion(bips));
             } else {
                 revert UnsupportedAction(action);
             }

src/base/DeltaResolver.sol

@@ -15,9 +15,9 @@ abstract contract DeltaResolver is ImmutableState {
     using SafeCast for *;
 
     /// @notice Emitted trying to settle a positive delta.
-    error IncorrectUseOfSettle();
+    error DeltaNotPositive(Currency currency);
     /// @notice Emitted trying to take a negative delta.
-    error IncorrectUseOfTake();
+    error DeltaNotNegative(Currency currency);
 
     /// @notice Take an amount of currency out of the PoolManager
     /// @param currency Currency to take
@@ -49,17 +49,23 @@ abstract contract DeltaResolver is ImmutableState {
     /// @param amount The number of tokens to send
     function _pay(Currency token, address payer, uint256 amount) internal virtual;
 
-    function _getFullSettleAmount(Currency currency) internal view returns (uint256 amount) {
+    /// @notice Obtain the full amount owed by this contract (negative delta)
+    /// @param currency Currency to get the delta for
+    /// @return amount The amount owed by this contract as a uint256
+    function _getFullDebt(Currency currency) internal view returns (uint256 amount) {
         int256 _amount = poolManager.currencyDelta(address(this), currency);
-        // If the amount is positive, it should be taken not settled for.
-        if (_amount > 0) revert IncorrectUseOfSettle();
+        // If the amount is negative, it should be settled not taken.
+        if (_amount > 0) revert DeltaNotNegative(currency);
         amount = uint256(-_amount);
     }
 
-    function _getFullTakeAmount(Currency currency) internal view returns (uint256 amount) {
+    /// @notice Obtain the full credit owed to this contract (positive delta)
+    /// @param currency Currency to get the delta for
+    /// @return amount The amount owed to this contract as a uint256
+    function _getFullCredit(Currency currency) internal view returns (uint256 amount) {
         int256 _amount = poolManager.currencyDelta(address(this), currency);
-        // If the amount is negative, it should be settled not taken.
-        if (_amount < 0) revert IncorrectUseOfTake();
+        // If the amount is negative, it should be taken not settled for.
+        if (_amount < 0) revert DeltaNotPositive(currency);
         amount = uint256(_amount);
     }
 
@@ -68,7 +74,7 @@ abstract contract DeltaResolver is ImmutableState {
         if (amount == Constants.CONTRACT_BALANCE) {
             return currency.balanceOfSelf();
         } else if (amount == Constants.OPEN_DELTA) {
-            return _getFullSettleAmount(currency);
+            return _getFullDebt(currency);
         }
         return amount;
     }
@@ -78,7 +84,7 @@ abstract contract DeltaResolver is ImmutableState {
         if (amount == Constants.CONTRACT_BALANCE) {
             return currency.balanceOfSelf().toUint128();
         } else if (amount == Constants.OPEN_DELTA) {
-            return _getFullTakeAmount(currency).toUint128();
+            return _getFullCredit(currency).toUint128();
         }
         return amount;
     }

src/interfaces/IPositionManager.sol

@@ -10,7 +10,6 @@ interface IPositionManager is INotifier {
     error NotApproved(address caller);
     error DeadlinePassed();
     error IncorrectPositionConfigForTokenId(uint256 tokenId);
-    error ClearExceedsMaxAmount(Currency currency, int256 amount, uint256 maxAmount);
 
     /// @notice Batches many liquidity modification calls to pool manager
     /// @param payload is an encoding of actions, and parameters for those actions

src/libraries/Actions.sol

@@ -29,7 +29,7 @@ library Actions {
     uint256 constant TAKE_PORTION = 0x14;
 
     uint256 constant CLOSE_CURRENCY = 0x15;
-    uint256 constant CLEAR = 0x16;
+    uint256 constant CLEAR_OR_TAKE = 0x16;
     uint256 constant SWEEP = 0x17;
 
     // minting/burning 6909s to close deltas

test/BaseActionsRouter.t.sol

@@ -77,7 +77,7 @@ contract BaseActionsRouterTest is Test, Deployers, GasSnapshot {
     function test_clear_suceeds() public {
         Plan memory plan = Planner.init();
         for (uint256 i = 0; i < 10; i++) {
-            plan.add(Actions.CLEAR, "");
+            plan.add(Actions.CLEAR_OR_TAKE, "");
         }
 
         assertEq(router.clearCount(), 0);

test/mocks/MockBaseActionsRouter.sol

@@ -34,7 +34,7 @@ contract MockBaseActionsRouter is BaseActionsRouter, ReentrancyLock {
         } else {
             if (action == Actions.SETTLE) _settle(params);
             else if (action == Actions.TAKE) _take(params);
-            else if (action == Actions.CLEAR) _clear(params);
+            else if (action == Actions.CLEAR_OR_TAKE) _clear(params);
             else if (action == Actions.MINT_6909) _mint6909(params);
             else if (action == Actions.BURN_6909) _burn6909(params);
             else revert UnsupportedAction(action);

test/position-managers/IncreaseLiquidity.t.sol

@@ -20,6 +20,7 @@ import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol";
 import {IERC20} from "forge-std/interfaces/IERC20.sol";
 
 import {PositionManager} from "../../src/PositionManager.sol";
+import {DeltaResolver} from "../../src/base/DeltaResolver.sol";
 import {PositionConfig} from "../../src/libraries/PositionConfig.sol";
 import {SlippageCheckLibrary} from "../../src/libraries/SlippageCheck.sol";
 import {IPositionManager} from "../../src/interfaces/IPositionManager.sol";
@@ -177,8 +178,8 @@ contract IncreaseLiquidityTest is Test, PosmTestSetup, Fuzzers {
             Actions.INCREASE_LIQUIDITY,
             abi.encode(tokenIdAlice, config, liquidityDelta, MAX_SLIPPAGE_INCREASE, MAX_SLIPPAGE_INCREASE, ZERO_BYTES)
         );
-        planner.add(Actions.CLEAR, abi.encode(config.poolKey.currency0, 18 wei)); // alice is willing to forfeit 18 wei
-        planner.add(Actions.CLEAR, abi.encode(config.poolKey.currency1, 18 wei));
+        planner.add(Actions.CLEAR_OR_TAKE, abi.encode(config.poolKey.currency0, 18 wei)); // alice is willing to forfeit 18 wei
+        planner.add(Actions.CLEAR_OR_TAKE, abi.encode(config.poolKey.currency1, 18 wei));
         bytes memory calls = planner.encode();
 
         vm.prank(alice);
@@ -280,8 +281,8 @@ contract IncreaseLiquidityTest is Test, PosmTestSetup, Fuzzers {
             Actions.INCREASE_LIQUIDITY,
             abi.encode(tokenIdAlice, config, liquidityDelta, MAX_SLIPPAGE_INCREASE, MAX_SLIPPAGE_INCREASE, ZERO_BYTES)
         );
-        planner.add(Actions.CLEAR, abi.encode(config.poolKey.currency0, 1 wei)); // alice is willing to forfeit 1 wei
-        planner.add(Actions.CLEAR, abi.encode(config.poolKey.currency1, 1 wei));
+        planner.add(Actions.CLEAR_OR_TAKE, abi.encode(config.poolKey.currency0, 1 wei)); // alice is willing to forfeit 1 wei
+        planner.add(Actions.CLEAR_OR_TAKE, abi.encode(config.poolKey.currency1, 1 wei));
         bytes memory calls = planner.encode();
 
         vm.prank(alice);
@@ -692,7 +693,8 @@ contract IncreaseLiquidityTest is Test, PosmTestSetup, Fuzzers {
         assertEq(currency1.balanceOf(address(this)), balanceBefore1 - amount1);
     }
 
-    function test_increaseLiquidity_clearExceeds_revert() public {
+    /// @dev if clearing exceeds the max amount, the amount is taken instead
+    function test_increaseLiquidity_clearExceedsThenTake() public {
         uint256 tokenId = lpm.nextTokenId();
         mint(config, 1000e18, address(this), ZERO_BYTES);
 
@@ -702,6 +704,7 @@ contract IncreaseLiquidityTest is Test, PosmTestSetup, Fuzzers {
 
         // calculate the amount of liquidity to add, using half of the proceeds
         uint256 amountToReinvest = amountToDonate / 2;
+        uint256 amountToReclaim = amountToDonate / 2; // expect to reclaim the other half of the fee revenue
         uint256 liquidityDelta = LiquidityAmounts.getLiquidityForAmounts(
             SQRT_PRICE_1_1,
             TickMath.getSqrtPriceAtTick(config.tickLower),
@@ -710,28 +713,33 @@ contract IncreaseLiquidityTest is Test, PosmTestSetup, Fuzzers {
             amountToReinvest
         );
 
+        // set the max-forfeit to less than the amount we expect to claim
+        uint256 maxClear = amountToReclaim - 2 wei;
+
         Plan memory planner = Planner.init();
         planner.add(
             Actions.INCREASE_LIQUIDITY,
             abi.encode(tokenId, config, liquidityDelta, MAX_SLIPPAGE_INCREASE, MAX_SLIPPAGE_INCREASE, ZERO_BYTES)
         );
-        planner.add(Actions.CLEAR, abi.encode(config.poolKey.currency0, amountToReinvest - 2 wei));
-        planner.add(Actions.CLEAR, abi.encode(config.poolKey.currency1, amountToReinvest - 2 wei));
+        planner.add(Actions.CLEAR_OR_TAKE, abi.encode(config.poolKey.currency0, maxClear));
+        planner.add(Actions.CLEAR_OR_TAKE, abi.encode(config.poolKey.currency1, maxClear));
         bytes memory calls = planner.encode();
 
-        // revert since we're forfeiting beyond the max tolerance
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IPositionManager.ClearExceedsMaxAmount.selector,
-                config.poolKey.currency0,
-                int256(amountToReinvest - 1 wei), // imprecision, PM expects us to collect half of the fees (minus 1 wei)
-                uint256(amountToReinvest - 2 wei) // the maximum amount we were willing to forfeit
-            )
-        );
+        uint256 balance0Before = currency0.balanceOf(address(this));
+        uint256 balance1Before = currency1.balanceOf(address(this));
+
+        // expect to take the excess, as it exceeds the amount to clear
         lpm.modifyLiquidities(calls, _deadline);
+        BalanceDelta delta = getLastDelta();
+
+        assertEq(uint128(delta.amount0()), amountToReclaim - 1 wei); // imprecision
+        assertEq(uint128(delta.amount1()), amountToReclaim - 1 wei);
+
+        assertEq(currency0.balanceOf(address(this)), balance0Before + amountToReclaim - 1 wei);
+        assertEq(currency1.balanceOf(address(this)), balance1Before + amountToReclaim - 1 wei);
     }
 
-    /// @dev clearing a negative delta reverts in core with SafeCastOverflow
+    /// @dev clearing a negative delta reverts
     function test_increaseLiquidity_clearNegative_revert() public {
         uint256 tokenId = lpm.nextTokenId();
         mint(config, 1000e18, address(this), ZERO_BYTES);
@@ -742,12 +750,12 @@ contract IncreaseLiquidityTest is Test, PosmTestSetup, Fuzzers {
             Actions.INCREASE_LIQUIDITY,
             abi.encode(tokenId, config, 100e18, MAX_SLIPPAGE_INCREASE, MAX_SLIPPAGE_INCREASE, ZERO_BYTES)
         );
-        planner.add(Actions.CLEAR, abi.encode(config.poolKey.currency0, type(uint256).max));
-        planner.add(Actions.CLEAR, abi.encode(config.poolKey.currency1, type(uint256).max));
+        planner.add(Actions.CLEAR_OR_TAKE, abi.encode(config.poolKey.currency0, type(uint256).max));
+        planner.add(Actions.CLEAR_OR_TAKE, abi.encode(config.poolKey.currency1, type(uint256).max));
         bytes memory calls = planner.encode();
 
-        // revert since we're forfeiting beyond the max tolerance
-        vm.expectRevert(SafeCast.SafeCastOverflow.selector);
+        // revert since we're trying to clear a negative delta
+        vm.expectRevert(abi.encodeWithSelector(DeltaResolver.DeltaNotPositive.selector, currency0));
         lpm.modifyLiquidities(calls, _deadline);
     }
 }

test/position-managers/PositionManager.gas.t.sol

@@ -263,8 +263,8 @@ contract PosMGasTest is Test, PosmTestSetup, GasSnapshot {
             Actions.INCREASE_LIQUIDITY,
             abi.encode(tokenIdAlice, config, liquidityDelta, MAX_SLIPPAGE_INCREASE, MAX_SLIPPAGE_INCREASE, ZERO_BYTES)
         );
-        planner.add(Actions.CLEAR, abi.encode(config.poolKey.currency0, halfTokensOwedAlice + 1 wei));
-        planner.add(Actions.CLEAR, abi.encode(config.poolKey.currency1, halfTokensOwedAlice + 1 wei));
+        planner.add(Actions.CLEAR_OR_TAKE, abi.encode(config.poolKey.currency0, halfTokensOwedAlice + 1 wei));
+        planner.add(Actions.CLEAR_OR_TAKE, abi.encode(config.poolKey.currency1, halfTokensOwedAlice + 1 wei));
         bytes memory calls = planner.encode();
 
         vm.prank(alice);