diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap index 632672052..2d76ace08 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap @@ -1 +1 @@ -123924 \ No newline at end of file +123923 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap index d52f99cb8..1263d1181 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap @@ -1 +1 @@ -123422 \ No newline at end of file +123420 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap index 71a5011c9..618bac703 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap @@ -1 +1 @@ -131003 \ No newline at end of file +131001 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap index c2f60cd71..5a059d8b3 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap @@ -1 +1 @@ -130500 \ No newline at end of file +130499 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_native.snap b/.forge-snapshots/PositionManager_collect_native.snap index 4d3da98ca..1536cdec7 100644 --- a/.forge-snapshots/PositionManager_collect_native.snap +++ b/.forge-snapshots/PositionManager_collect_native.snap @@ -1 +1 @@ -142433 \ No newline at end of file +142431 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_sameRange.snap b/.forge-snapshots/PositionManager_collect_sameRange.snap index e65741c1f..de5d67d1a 100644 --- a/.forge-snapshots/PositionManager_collect_sameRange.snap +++ b/.forge-snapshots/PositionManager_collect_sameRange.snap @@ -1 +1 @@ -151281 \ No newline at end of file +151279 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_withClose.snap b/.forge-snapshots/PositionManager_collect_withClose.snap index e65741c1f..de5d67d1a 100644 --- a/.forge-snapshots/PositionManager_collect_withClose.snap +++ b/.forge-snapshots/PositionManager_collect_withClose.snap @@ -1 +1 @@ -151281 \ No newline at end of file +151279 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_withTakePair.snap b/.forge-snapshots/PositionManager_collect_withTakePair.snap index e1d82d967..4b2083fda 100644 --- a/.forge-snapshots/PositionManager_collect_withTakePair.snap +++ b/.forge-snapshots/PositionManager_collect_withTakePair.snap @@ -1 +1 @@ -150641 \ No newline at end of file +150639 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap index 40772dc20..ede22497d 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap @@ -1 +1 @@ -109421 \ No newline at end of file +109420 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap index f7cbc3b60..9f32a7996 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap @@ -1 +1 @@ -116824 \ No newline at end of file +116822 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap index 8ec8cc23c..ac2386537 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap @@ -1 +1 @@ -116184 \ No newline at end of file +116182 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap index 6ca9f902e..dc105d6a6 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap @@ -1 +1 @@ -135078 \ No newline at end of file +135076 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap index b4e75fea4..784739a55 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap @@ -1 +1 @@ -127817 \ No newline at end of file +127816 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap index 05acb4ce7..3ff1bbec5 100644 --- a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap +++ b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap @@ -1 +1 @@ -129540 \ No newline at end of file +129538 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_take_take.snap b/.forge-snapshots/PositionManager_decrease_take_take.snap index 34d197ce2..9099ea8e3 100644 --- a/.forge-snapshots/PositionManager_decrease_take_take.snap +++ b/.forge-snapshots/PositionManager_decrease_take_take.snap @@ -1 +1 @@ -117357 \ No newline at end of file +117355 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap index 97c4af609..4a9005d49 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap @@ -1 +1 @@ -155531 \ No newline at end of file +155504 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap index db4bd78fa..7a94efabc 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap @@ -1 +1 @@ -154533 \ No newline at end of file +154506 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap index 6058eccca..30541c563 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap @@ -1 +1 @@ -137331 \ No newline at end of file +137304 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap index 09f17e7ee..e35f36c43 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap @@ -1 +1 @@ -133676 \ No newline at end of file +133649 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap index 5d94cc32a..1f1f4be2c 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap @@ -1 +1 @@ -174592 \ No newline at end of file +174565 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap b/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap index 1ba47495a..ff2c4bd93 100644 --- a/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap +++ b/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap @@ -1 +1 @@ -144548 \ No newline at end of file +144521 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_native.snap b/.forge-snapshots/PositionManager_mint_native.snap index 0deae34bb..c4b5a8375 100644 --- a/.forge-snapshots/PositionManager_mint_native.snap +++ b/.forge-snapshots/PositionManager_mint_native.snap @@ -1 +1 @@ -341053 \ No newline at end of file +341026 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap index 3c36d95dd..3aa688240 100644 --- a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap +++ b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap @@ -1 +1 @@ -349545 \ No newline at end of file +349518 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap index 8ff987d61..5af5865cc 100644 --- a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap +++ b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap @@ -1 +1 @@ -348847 \ No newline at end of file +348820 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap index e4b71faf1..26b30309b 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap @@ -1 +1 @@ -319035 \ No newline at end of file +319008 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap index 604fa0a7a..a45fcbb16 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap @@ -1 +1 @@ -319677 \ No newline at end of file +319650 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_sameRange.snap b/.forge-snapshots/PositionManager_mint_sameRange.snap index 3d62eb22d..3eb2b8387 100644 --- a/.forge-snapshots/PositionManager_mint_sameRange.snap +++ b/.forge-snapshots/PositionManager_mint_sameRange.snap @@ -1 +1 @@ -245259 \ No newline at end of file +245232 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap b/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap index a1935f6a3..8da3d5725 100644 --- a/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap +++ b/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap @@ -1 +1 @@ -375077 \ No newline at end of file +375050 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap index 81609fe56..782f7f591 100644 --- a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap +++ b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap @@ -1 +1 @@ -325053 \ No newline at end of file +325026 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_withClose.snap b/.forge-snapshots/PositionManager_mint_withClose.snap index 201bbccf9..c3ad29224 100644 --- a/.forge-snapshots/PositionManager_mint_withClose.snap +++ b/.forge-snapshots/PositionManager_mint_withClose.snap @@ -1 +1 @@ -376353 \ No newline at end of file +376326 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_withSettlePair.snap b/.forge-snapshots/PositionManager_mint_withSettlePair.snap index 6bacbc55c..22235296f 100644 --- a/.forge-snapshots/PositionManager_mint_withSettlePair.snap +++ b/.forge-snapshots/PositionManager_mint_withSettlePair.snap @@ -1 +1 @@ -375493 \ No newline at end of file +375466 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap index 7d7ac2ca0..f7ee51aa0 100644 --- a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap +++ b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap @@ -1 +1 @@ -420802 \ No newline at end of file +420775 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_Bytecode.snap b/.forge-snapshots/V4Router_Bytecode.snap index 86c2e0ea7..f2faf802d 100644 --- a/.forge-snapshots/V4Router_Bytecode.snap +++ b/.forge-snapshots/V4Router_Bytecode.snap @@ -1 +1 @@ -8394 \ No newline at end of file +8596 \ No newline at end of file diff --git a/src/PositionManager.sol b/src/PositionManager.sol index e81e32d70..9c1533479 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -24,7 +24,7 @@ import {Actions} from "./libraries/Actions.sol"; import {Notifier} from "./base/Notifier.sol"; import {CalldataDecoder} from "./libraries/CalldataDecoder.sol"; import {Permit2Forwarder} from "./base/Permit2Forwarder.sol"; -import {SlippageCheckLibrary} from "./libraries/SlippageCheck.sol"; +import {SlippageCheck} from "./libraries/SlippageCheck.sol"; import {PositionConfigId, PositionConfigIdLibrary} from "./libraries/PositionConfigId.sol"; // 444444444 @@ -111,7 +111,7 @@ contract PositionManager is using SafeCast for uint256; using SafeCast for int256; using CalldataDecoder for bytes; - using SlippageCheckLibrary for BalanceDelta; + using SlippageCheck for BalanceDelta; using PositionConfigIdLibrary for PositionConfigId; /// @inheritdoc IPositionManager @@ -134,7 +134,7 @@ contract PositionManager is /// @notice Reverts if the deadline has passed /// @param deadline The timestamp at which the call is no longer valid, passed in by the caller modifier checkDeadline(uint256 deadline) { - if (block.timestamp > deadline) revert DeadlinePassed(); + if (block.timestamp > deadline) revert DeadlinePassed(deadline); _; } @@ -414,7 +414,6 @@ contract PositionManager is // implementation of abstract function DeltaResolver._pay function _pay(Currency currency, address payer, uint256 amount) internal override { if (payer == address(this)) { - // TODO: currency is guaranteed to not be eth so the native check in transfer is not optimal. currency.transfer(address(poolManager), amount); } else { permit2.transferFrom(payer, address(poolManager), uint160(amount), Currency.unwrap(currency)); diff --git a/src/V4Router.sol b/src/V4Router.sol index 22fbea4b7..c14ad76a7 100644 --- a/src/V4Router.sol +++ b/src/V4Router.sol @@ -60,13 +60,13 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver { } else if (action == Actions.SETTLE_ALL) { (Currency currency, uint256 maxAmount) = params.decodeCurrencyAndUint256(); uint256 amount = _getFullDebt(currency); - if (amount > maxAmount) revert V4TooMuchRequested(); + if (amount > maxAmount) revert V4TooMuchRequested(maxAmount, amount); _settle(currency, msgSender(), amount); return; } else if (action == Actions.TAKE_ALL) { (Currency currency, uint256 minAmount) = params.decodeCurrencyAndUint256(); uint256 amount = _getFullCredit(currency); - if (amount < minAmount) revert V4TooLittleReceived(); + if (amount < minAmount) revert V4TooLittleReceived(minAmount, amount); _take(currency, msgSender(), amount); return; } else if (action == Actions.SETTLE) { @@ -95,7 +95,7 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver { uint128 amountOut = _swap( params.poolKey, params.zeroForOne, int256(-int128(amountIn)), params.sqrtPriceLimitX96, params.hookData ).toUint128(); - if (amountOut < params.amountOutMinimum) revert V4TooLittleReceived(); + if (amountOut < params.amountOutMinimum) revert V4TooLittleReceived(params.amountOutMinimum, amountOut); } function _swapExactInput(IV4Router.ExactInputParams calldata params) private { @@ -118,7 +118,7 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver { currencyIn = pathKey.intermediateCurrency; } - if (amountOut < params.amountOutMinimum) revert V4TooLittleReceived(); + if (amountOut < params.amountOutMinimum) revert V4TooLittleReceived(params.amountOutMinimum, amountOut); } } @@ -132,7 +132,7 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver { params.hookData ) ).toUint128(); - if (amountIn > params.amountInMaximum) revert V4TooMuchRequested(); + if (amountIn > params.amountInMaximum) revert V4TooMuchRequested(params.amountInMaximum, amountIn); } function _swapExactOutput(IV4Router.ExactOutputParams calldata params) private { @@ -153,7 +153,7 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver { amountOut = amountIn; currencyOut = pathKey.intermediateCurrency; } - if (amountIn > params.amountInMaximum) revert V4TooMuchRequested(); + if (amountIn > params.amountInMaximum) revert V4TooMuchRequested(params.amountInMaximum, amountIn); } } diff --git a/src/base/EIP712_v4.sol b/src/base/EIP712_v4.sol index 61a5ad464..e66261aa5 100644 --- a/src/base/EIP712_v4.sol +++ b/src/base/EIP712_v4.sol @@ -15,8 +15,8 @@ contract EIP712_v4 is IEIP712_v4 { uint256 private immutable _CACHED_CHAIN_ID; bytes32 private immutable _HASHED_NAME; - /// @dev equal to keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)") - bytes32 private constant _TYPE_HASH = 0x8cad95687ba82c2ce50e74f7b754645e5117c3a5bec8151c0726d5857980a866; + bytes32 private constant _TYPE_HASH = + keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); constructor(string memory name) { _HASHED_NAME = keccak256(bytes(name)); diff --git a/src/base/ERC721Permit_v4.sol b/src/base/ERC721Permit_v4.sol index 7b2e7c00c..aab76f7c2 100644 --- a/src/base/ERC721Permit_v4.sol +++ b/src/base/ERC721Permit_v4.sol @@ -72,7 +72,7 @@ abstract contract ERC721Permit_v4 is ERC721, IERC721Permit_v4, EIP712_v4, Unorde /// @notice Change or reaffirm the approved address for an NFT /// @dev override Solmate's ERC721 approve so approve() and permit() share the _approve method - /// The zero address indicates there is no approved address + /// Passing a spender address of zero can be used to remove any outstanding approvals /// Throws error unless `msg.sender` is the current NFT owner, /// or an authorized operator of the current owner. /// @param spender The new approved NFT controller diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol index 3e52d9a72..2992b6a8b 100644 --- a/src/base/Notifier.sol +++ b/src/base/Notifier.sol @@ -15,16 +15,10 @@ abstract contract Notifier is INotifier { using CustomRevert for bytes4; using PositionConfigIdLibrary for PositionConfigId; - error AlreadySubscribed(address subscriber); - - event Subscribed(uint256 tokenId, address subscriber); - event Unsubscribed(uint256 tokenId, address subscriber); - ISubscriber private constant NO_SUBSCRIBER = ISubscriber(address(0)); // a percentage of the block.gaslimit denoted in BPS, used as the gas limit for subscriber calls - // 100 bps is 1% - // at 30M gas, the limit is 300K + // 100 bps is 1%, at 30M gas, the limit is 300K uint256 private constant BLOCK_LIMIT_BPS = 100; /// @inheritdoc INotifier @@ -46,7 +40,7 @@ abstract contract Notifier is INotifier { _positionConfigs(tokenId).setSubscribe(); ISubscriber _subscriber = subscriber[tokenId]; - if (_subscriber != NO_SUBSCRIBER) revert AlreadySubscribed(address(_subscriber)); + if (_subscriber != NO_SUBSCRIBER) revert AlreadySubscribed(tokenId, address(_subscriber)); subscriber[tokenId] = ISubscriber(newSubscriber); bool success = _call(newSubscriber, abi.encodeCall(ISubscriber.notifySubscribe, (tokenId, config, data))); @@ -55,7 +49,7 @@ abstract contract Notifier is INotifier { Wrap__SubscriptionReverted.selector.bubbleUpAndRevertWith(newSubscriber); } - emit Subscribed(tokenId, newSubscriber); + emit Subscription(tokenId, newSubscriber); } /// @inheritdoc INotifier @@ -74,10 +68,12 @@ abstract contract Notifier is INotifier { delete subscriber[tokenId]; + // A gas limit and a try-catch block are used to protect users from a malicious subscriber. + // Users should always be able to unsubscribe, not matter how the subscriber behaves. uint256 subscriberGasLimit = block.gaslimit.calculatePortion(BLOCK_LIMIT_BPS); try _subscriber.notifyUnsubscribe{gas: subscriberGasLimit}(tokenId, config) {} catch {} - emit Unsubscribed(tokenId, address(_subscriber)); + emit Unsubscription(tokenId, address(_subscriber)); } function _notifyModifyLiquidity( diff --git a/src/base/Permit2Forwarder.sol b/src/base/Permit2Forwarder.sol index d6f28ccce..12e378aae 100644 --- a/src/base/Permit2Forwarder.sol +++ b/src/base/Permit2Forwarder.sol @@ -8,6 +8,8 @@ import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol" contract Permit2Forwarder { IAllowanceTransfer public immutable permit2; + error Wrap__Permit2Reverted(address _permit2, bytes reason); + constructor(IAllowanceTransfer _permit2) { permit2 = _permit2; } @@ -17,8 +19,13 @@ contract Permit2Forwarder { function permit(address owner, IAllowanceTransfer.PermitSingle calldata permitSingle, bytes calldata signature) external payable + returns (bytes memory err) { - permit2.permit(owner, permitSingle, signature); + // use try/catch in case an actor front-runs the permit, which would DOS multicalls + try permit2.permit(owner, permitSingle, signature) {} + catch (bytes memory reason) { + err = abi.encodeWithSelector(Wrap__Permit2Reverted.selector, address(permit2), reason); + } } /// @notice allows forwarding batch permits to permit2 @@ -26,7 +33,12 @@ contract Permit2Forwarder { function permitBatch(address owner, IAllowanceTransfer.PermitBatch calldata _permitBatch, bytes calldata signature) external payable + returns (bytes memory err) { - permit2.permit(owner, _permitBatch, signature); + // use try/catch in case an actor front-runs the permit, which would DOS multicalls + try permit2.permit(owner, _permitBatch, signature) {} + catch (bytes memory reason) { + err = abi.encodeWithSelector(Wrap__Permit2Reverted.selector, address(permit2), reason); + } } } diff --git a/src/base/SafeCallback.sol b/src/base/SafeCallback.sol index 134bf5642..ab80cd845 100644 --- a/src/base/SafeCallback.sol +++ b/src/base/SafeCallback.sol @@ -10,13 +10,13 @@ abstract contract SafeCallback is ImmutableState, IUnlockCallback { constructor(IPoolManager _poolManager) ImmutableState(_poolManager) {} - modifier onlyByPoolManager() { + modifier onlyPoolManager() { if (msg.sender != address(poolManager)) revert NotPoolManager(); _; } - /// @dev We force the onlyByPoolManager modifier by exposing a virtual function after the onlyByPoolManager check. - function unlockCallback(bytes calldata data) external onlyByPoolManager returns (bytes memory) { + /// @dev We force the onlyPoolManager modifier by exposing a virtual function after the onlyPoolManager check. + function unlockCallback(bytes calldata data) external onlyPoolManager returns (bytes memory) { return _unlockCallback(data); } diff --git a/src/interfaces/IMulticall_v4.sol b/src/interfaces/IMulticall_v4.sol index 2a88c0970..1d053a97d 100644 --- a/src/interfaces/IMulticall_v4.sol +++ b/src/interfaces/IMulticall_v4.sol @@ -5,7 +5,8 @@ pragma solidity ^0.8.0; /// @notice Enables calling multiple methods in a single call to the contract interface IMulticall_v4 { /// @notice Call multiple functions in the current contract and return the data from all of them if they all succeed - /// @dev The `msg.value` should not be trusted for any method callable from multicall. + /// @dev The `msg.value` is passed onto all subcalls, even if a previous subcall has consumed the ether. + /// Subcalls can instead use `address(this).value` to see the available ETH, and consume it using {value: x}. /// @param data The encoded function data for each of the calls to make to this contract /// @return results The results from each of the calls passed in via data function multicall(bytes[] calldata data) external payable returns (bytes[] memory results); diff --git a/src/interfaces/INotifier.sol b/src/interfaces/INotifier.sol index 0dc7be3c9..885a93289 100644 --- a/src/interfaces/INotifier.sol +++ b/src/interfaces/INotifier.sol @@ -12,6 +12,13 @@ interface INotifier { error Wrap__ModifyLiquidityNotificationReverted(address subscriber, bytes reason); /// @notice Wraps the revert message of the subscriber contract on a reverting transfer notification error Wrap__TransferNotificationReverted(address subscriber, bytes reason); + /// @notice Thrown when a tokenId already has a subscriber + error AlreadySubscribed(uint256 tokenId, address subscriber); + + /// @notice Emitted on a successful call to subscribe + event Subscription(uint256 indexed tokenId, address indexed subscriber); + /// @notice Emitted on a successful call to unsubscribe + event Unsubscription(uint256 indexed tokenId, address indexed subscriber); /// @notice Returns the subscriber for a respective position /// @param tokenId the ERC721 tokenId diff --git a/src/interfaces/IPositionManager.sol b/src/interfaces/IPositionManager.sol index b6f88cfcf..8f7a59c63 100644 --- a/src/interfaces/IPositionManager.sol +++ b/src/interfaces/IPositionManager.sol @@ -7,7 +7,7 @@ import {INotifier} from "./INotifier.sol"; interface IPositionManager is INotifier { error NotApproved(address caller); - error DeadlinePassed(); + error DeadlinePassed(uint256 deadline); error IncorrectPositionConfigForTokenId(uint256 tokenId); event MintPosition(uint256 indexed tokenId, PositionConfig config); diff --git a/src/interfaces/IV4Router.sol b/src/interfaces/IV4Router.sol index 40a995e8e..13ed4775f 100644 --- a/src/interfaces/IV4Router.sol +++ b/src/interfaces/IV4Router.sol @@ -9,9 +9,9 @@ import {PathKey} from "../libraries/PathKey.sol"; /// @notice Interface containing all the structs and errors for different v4 swap types interface IV4Router { /// @notice Emitted when an exactInput swap does not receive its minAmountOut - error V4TooLittleReceived(); + error V4TooLittleReceived(uint256 minAmountOutReceived, uint256 amountReceived); /// @notice Emitted when an exactOutput is asked for more than its maxAmountIn - error V4TooMuchRequested(); + error V4TooMuchRequested(uint256 maxAmountInRequested, uint256 amountRequested); /// @notice Parameters for a single-hop exact-input swap struct ExactInputSingleParams { diff --git a/src/libraries/SlippageCheck.sol b/src/libraries/SlippageCheck.sol index 00d9d23b3..860dea374 100644 --- a/src/libraries/SlippageCheck.sol +++ b/src/libraries/SlippageCheck.sol @@ -6,11 +6,11 @@ import {SafeCastTemp} from "./SafeCast.sol"; /// @title Slippage Check Library /// @notice a library for checking if a delta exceeds a maximum ceiling or fails to meet a minimum floor -library SlippageCheckLibrary { +library SlippageCheck { using SafeCastTemp for int128; - error MaximumAmountExceeded(); - error MinimumAmountInsufficient(); + error MaximumAmountExceeded(uint128 maximumAmount, uint128 amountRequested); + error MinimumAmountInsufficient(uint128 minimumAmount, uint128 amountReceived); /// @notice Revert if one or both deltas does not meet a minimum output /// @param delta The principal amount of tokens to be removed, does not include any fees accrued @@ -22,8 +22,11 @@ library SlippageCheckLibrary { // However, on pools where hooks can return deltas on modify liquidity, it is possible for a returned delta to be negative. // Because we use SafeCast, this will revert in those cases when the delta is negative. // This means this contract will NOT support pools where the hook returns a negative delta on burn/decrease. - if (delta.amount0().toUint128() < amount0Min || delta.amount1().toUint128() < amount1Min) { - revert MinimumAmountInsufficient(); + if (delta.amount0().toUint128() < amount0Min) { + revert MinimumAmountInsufficient(amount0Min, delta.amount0().toUint128()); + } + if (delta.amount1().toUint128() < amount1Min) { + revert MinimumAmountInsufficient(amount1Min, delta.amount1().toUint128()); } } @@ -38,9 +41,9 @@ library SlippageCheckLibrary { // Thus, we only cast the delta if it is guaranteed to be negative. // And we do NOT revert in the positive delta case. Since a positive delta means the hook is crediting tokens to the user for minting/increasing liquidity, we do not check slippage. // This means this contract will NOT support _positive_ slippage checks (minAmountOut checks) on pools where the hook returns a positive delta on mint/increase. - if ( - delta.amount0() < 0 && amount0Max < uint128(-delta.amount0()) - || delta.amount1() < 0 && amount1Max < uint128(-delta.amount1()) - ) revert MaximumAmountExceeded(); + int128 amount0 = delta.amount0(); + int128 amount1 = delta.amount1(); + if (amount0 < 0 && amount0Max < uint128(-amount0)) revert MaximumAmountExceeded(amount0Max, uint128(-amount0)); + if (amount1 < 0 && amount1Max < uint128(-amount1)) revert MaximumAmountExceeded(amount1Max, uint128(-amount1)); } } diff --git a/test/position-managers/IncreaseLiquidity.t.sol b/test/position-managers/IncreaseLiquidity.t.sol index 1feef6bc1..f4d103e11 100644 --- a/test/position-managers/IncreaseLiquidity.t.sol +++ b/test/position-managers/IncreaseLiquidity.t.sol @@ -22,7 +22,7 @@ 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 {SlippageCheck} from "../../src/libraries/SlippageCheck.sol"; import {IPositionManager} from "../../src/interfaces/IPositionManager.sol"; import {Actions} from "../../src/libraries/Actions.sol"; import {Planner, Plan} from "../shared/Planner.sol"; @@ -540,9 +540,16 @@ contract IncreaseLiquidityTest is Test, PosmTestSetup, Fuzzers { uint256 tokenId = lpm.nextTokenId(); mint(config, 100e18, ActionConstants.MSG_SENDER, ZERO_BYTES); + uint128 newLiquidity = 100e18; + (uint256 amount0,) = LiquidityAmounts.getAmountsForLiquidity( + SQRT_PRICE_1_1, + TickMath.getSqrtPriceAtTick(config.tickLower), + TickMath.getSqrtPriceAtTick(config.tickUpper), + newLiquidity + ); // revert since amount0Max is too low - bytes memory calls = getIncreaseEncoded(tokenId, config, 100e18, 1 wei, type(uint128).max, ZERO_BYTES); - vm.expectRevert(SlippageCheckLibrary.MaximumAmountExceeded.selector); + bytes memory calls = getIncreaseEncoded(tokenId, config, newLiquidity, 1 wei, type(uint128).max, ZERO_BYTES); + vm.expectRevert(abi.encodeWithSelector(SlippageCheck.MaximumAmountExceeded.selector, 1 wei, amount0 + 1)); lpm.modifyLiquidities(calls, _deadline); } @@ -551,9 +558,16 @@ contract IncreaseLiquidityTest is Test, PosmTestSetup, Fuzzers { uint256 tokenId = lpm.nextTokenId(); mint(config, 100e18, ActionConstants.MSG_SENDER, ZERO_BYTES); + uint128 newLiquidity = 100e18; + (, uint256 amount1) = LiquidityAmounts.getAmountsForLiquidity( + SQRT_PRICE_1_1, + TickMath.getSqrtPriceAtTick(config.tickLower), + TickMath.getSqrtPriceAtTick(config.tickUpper), + newLiquidity + ); // revert since amount1Max is too low - bytes memory calls = getIncreaseEncoded(tokenId, config, 100e18, type(uint128).max, 1 wei, ZERO_BYTES); - vm.expectRevert(SlippageCheckLibrary.MaximumAmountExceeded.selector); + bytes memory calls = getIncreaseEncoded(tokenId, config, newLiquidity, type(uint128).max, 1 wei, ZERO_BYTES); + vm.expectRevert(abi.encodeWithSelector(SlippageCheck.MaximumAmountExceeded.selector, 1 wei, amount1 + 1)); lpm.modifyLiquidities(calls, _deadline); } @@ -601,7 +615,9 @@ contract IncreaseLiquidityTest is Test, PosmTestSetup, Fuzzers { swap(key, true, -10e18, ZERO_BYTES); bytes memory calls = getIncreaseEncoded(tokenId, config, newLiquidity, slippage, slippage, ZERO_BYTES); - vm.expectRevert(SlippageCheckLibrary.MaximumAmountExceeded.selector); + vm.expectRevert( + abi.encodeWithSelector(SlippageCheck.MaximumAmountExceeded.selector, slippage, 299996249439153403) + ); lpm.modifyLiquidities(calls, _deadline); } diff --git a/test/position-managers/PositionManager.multicall.t.sol b/test/position-managers/PositionManager.multicall.t.sol index 73c7e6587..9264c28e6 100644 --- a/test/position-managers/PositionManager.multicall.t.sol +++ b/test/position-managers/PositionManager.multicall.t.sol @@ -46,6 +46,8 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest address bob; // bob used for permit2 signature tests uint256 bobPK; + address charlie; // charlie will NOT approve posm in setup() + uint256 charliePK; Permit2Forwarder permit2Forwarder; @@ -54,6 +56,9 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest uint48 permitExpiration = uint48(block.timestamp + 10e18); uint48 permitNonce = 0; + // redefine error from permit2/src/PermitErrors.sol since its hard-pinned to a solidity version + error InvalidNonce(); + bytes32 PERMIT2_DOMAIN_SEPARATOR; PositionConfig config; @@ -61,6 +66,7 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest function setUp() public { (alice, alicePK) = makeAddrAndKey("ALICE"); (bob, bobPK) = makeAddrAndKey("BOB"); + (charlie, charliePK) = makeAddrAndKey("CHARLIE"); deployFreshManagerAndRouters(); deployMintAndApprove2Currencies(); @@ -78,6 +84,13 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest seedBalance(bob); approvePosmFor(bob); + + // do not approve posm for charlie, but approve permit2 for allowance transfer + seedBalance(charlie); + vm.startPrank(charlie); + IERC20(Currency.unwrap(currency0)).approve(address(permit2), type(uint256).max); + IERC20(Currency.unwrap(currency1)).approve(address(permit2), type(uint256).max); + vm.stopPrank(); } function test_multicall_initializePool_mint() public { @@ -174,7 +187,6 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest bytes[] memory calls = new bytes[](1); calls[0] = abi.encodeWithSelector(IPositionManager.modifyLiquidities.selector, actions, _deadline); - address charlie = makeAddr("CHARLIE"); vm.startPrank(charlie); vm.expectRevert(abi.encodeWithSelector(IPositionManager.NotApproved.selector, charlie)); lpm.multicall(calls); @@ -360,4 +372,136 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest assertEq(liquidity, 10e18); assertEq(lpm.ownerOf(tokenId), bob); } + + /// @notice test that a front-ran permit does not fail a multicall with permit + function test_multicall_permit_frontrun_suceeds() public { + config = PositionConfig({ + poolKey: key, + tickLower: TickMath.minUsableTick(key.tickSpacing), + tickUpper: TickMath.maxUsableTick(key.tickSpacing) + }); + + // Charlie signs permit for the two tokens + IAllowanceTransfer.PermitSingle memory permit0 = + defaultERC20PermitAllowance(Currency.unwrap(currency0), permitAmount, permitExpiration, permitNonce); + permit0.spender = address(lpm); + bytes memory sig0 = getPermitSignature(permit0, charliePK, PERMIT2_DOMAIN_SEPARATOR); + + IAllowanceTransfer.PermitSingle memory permit1 = + defaultERC20PermitAllowance(Currency.unwrap(currency1), permitAmount, permitExpiration, permitNonce); + permit1.spender = address(lpm); + bytes memory sig1 = getPermitSignature(permit1, charliePK, PERMIT2_DOMAIN_SEPARATOR); + + // bob front-runs the permits + vm.startPrank(bob); + lpm.permit(charlie, permit0, sig0); + lpm.permit(charlie, permit1, sig1); + vm.stopPrank(); + + // bob's front-run was successful + (uint160 _amount, uint48 _expiration, uint48 _nonce) = + permit2.allowance(charlie, Currency.unwrap(currency0), address(lpm)); + assertEq(_amount, permitAmount); + assertEq(_expiration, permitExpiration); + assertEq(_nonce, permitNonce + 1); + (uint160 _amount1, uint48 _expiration1, uint48 _nonce1) = + permit2.allowance(charlie, Currency.unwrap(currency1), address(lpm)); + assertEq(_amount1, permitAmount); + assertEq(_expiration1, permitExpiration); + assertEq(_nonce1, permitNonce + 1); + + // charlie tries to mint an LP token with multicall(permit, permit, mint) + bytes[] memory calls = new bytes[](3); + calls[0] = abi.encodeWithSelector(Permit2Forwarder(lpm).permit.selector, charlie, permit0, sig0); + calls[1] = abi.encodeWithSelector(Permit2Forwarder(lpm).permit.selector, charlie, permit1, sig1); + bytes memory mintCall = getMintEncoded(config, 10e18, charlie, ZERO_BYTES); + calls[2] = abi.encodeWithSelector(IPositionManager.modifyLiquidities.selector, mintCall, _deadline); + + uint256 tokenId = lpm.nextTokenId(); + vm.expectRevert(); + lpm.ownerOf(tokenId); // token does not exist + + bytes[] memory results = lpm.multicall(calls); + assertEq( + results[0], + abi.encode( + abi.encodeWithSelector( + Permit2Forwarder.Wrap__Permit2Reverted.selector, + address(permit2), + abi.encodeWithSelector(InvalidNonce.selector) + ) + ) + ); + assertEq( + results[1], + abi.encode( + abi.encodeWithSelector( + Permit2Forwarder.Wrap__Permit2Reverted.selector, + address(permit2), + abi.encodeWithSelector(InvalidNonce.selector) + ) + ) + ); + + assertEq(lpm.ownerOf(tokenId), charlie); + } + + /// @notice test that a front-ran permitBatch does not fail a multicall with permitBatch + function test_multicall_permitBatch_frontrun_suceeds() public { + config = PositionConfig({ + poolKey: key, + tickLower: TickMath.minUsableTick(key.tickSpacing), + tickUpper: TickMath.maxUsableTick(key.tickSpacing) + }); + + // Charlie signs permitBatch for the two tokens + address[] memory tokens = new address[](2); + tokens[0] = Currency.unwrap(currency0); + tokens[1] = Currency.unwrap(currency1); + + IAllowanceTransfer.PermitBatch memory permit = + defaultERC20PermitBatchAllowance(tokens, permitAmount, permitExpiration, permitNonce); + permit.spender = address(lpm); + bytes memory sig = getPermitBatchSignature(permit, charliePK, PERMIT2_DOMAIN_SEPARATOR); + + // bob front-runs the permits + vm.prank(bob); + lpm.permitBatch(charlie, permit, sig); + + // bob's front-run was successful + (uint160 _amount, uint48 _expiration, uint48 _nonce) = + permit2.allowance(charlie, Currency.unwrap(currency0), address(lpm)); + assertEq(_amount, permitAmount); + assertEq(_expiration, permitExpiration); + assertEq(_nonce, permitNonce + 1); + (uint160 _amount1, uint48 _expiration1, uint48 _nonce1) = + permit2.allowance(charlie, Currency.unwrap(currency1), address(lpm)); + assertEq(_amount1, permitAmount); + assertEq(_expiration1, permitExpiration); + assertEq(_nonce1, permitNonce + 1); + + // charlie tries to mint an LP token with multicall(permitBatch, mint) + bytes[] memory calls = new bytes[](2); + calls[0] = abi.encodeWithSelector(Permit2Forwarder(lpm).permitBatch.selector, charlie, permit, sig); + bytes memory mintCall = getMintEncoded(config, 10e18, charlie, ZERO_BYTES); + calls[1] = abi.encodeWithSelector(IPositionManager.modifyLiquidities.selector, mintCall, _deadline); + + uint256 tokenId = lpm.nextTokenId(); + vm.expectRevert(); + lpm.ownerOf(tokenId); // token does not exist + + bytes[] memory results = lpm.multicall(calls); + assertEq( + results[0], + abi.encode( + abi.encodeWithSelector( + Permit2Forwarder.Wrap__Permit2Reverted.selector, + address(permit2), + abi.encodeWithSelector(InvalidNonce.selector) + ) + ) + ); + + assertEq(lpm.ownerOf(tokenId), charlie); + } } diff --git a/test/position-managers/PositionManager.notifier.t.sol b/test/position-managers/PositionManager.notifier.t.sol index 1ab08e3e5..ea7931152 100644 --- a/test/position-managers/PositionManager.notifier.t.sol +++ b/test/position-managers/PositionManager.notifier.t.sol @@ -97,6 +97,25 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { assertEq(sub.notifySubscribeCount(), 1); } + function test_subscribe_revertsWithAlreadySubscribed() public { + uint256 tokenId = lpm.nextTokenId(); + mint(config, 100e18, alice, ZERO_BYTES); + + // approve this contract to operate on alices liq + vm.startPrank(alice); + lpm.approve(address(this), tokenId); + vm.stopPrank(); + + // successfully subscribe + lpm.subscribe(tokenId, config, address(sub), ZERO_BYTES); + assertEq(lpm.hasSubscriber(tokenId), true); + assertEq(address(lpm.subscriber(tokenId)), address(sub)); + assertEq(sub.notifySubscribeCount(), 1); + + vm.expectRevert(abi.encodeWithSelector(INotifier.AlreadySubscribed.selector, tokenId, sub)); + lpm.subscribe(tokenId, config, address(2), ZERO_BYTES); + } + function test_notifyModifyLiquidity_succeeds() public { uint256 tokenId = lpm.nextTokenId(); mint(config, 100e18, alice, ZERO_BYTES); diff --git a/test/position-managers/PositionManager.t.sol b/test/position-managers/PositionManager.t.sol index ab4be3915..7e7ea43f0 100644 --- a/test/position-managers/PositionManager.t.sol +++ b/test/position-managers/PositionManager.t.sol @@ -25,7 +25,7 @@ import {Actions} from "../../src/libraries/Actions.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 {SlippageCheck} from "../../src/libraries/SlippageCheck.sol"; import {BaseActionsRouter} from "../../src/base/BaseActionsRouter.sol"; import {ActionConstants} from "../../src/libraries/ActionConstants.sol"; @@ -61,6 +61,16 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { approvePosmFor(alice); } + function test_modifyLiquidities_reverts_deadlinePassed() public { + PositionConfig memory config = PositionConfig({poolKey: key, tickLower: 0, tickUpper: 60}); + bytes memory calls = getMintEncoded(config, 1e18, ActionConstants.MSG_SENDER, ""); + + uint256 deadline = vm.getBlockTimestamp() - 1; + + vm.expectRevert(abi.encodeWithSelector(IPositionManager.DeadlinePassed.selector, deadline)); + lpm.modifyLiquidities(calls, deadline); + } + function test_modifyLiquidities_reverts_mismatchedLengths() public { Plan memory planner = Planner.init(); planner.add(Actions.MINT_POSITION, abi.encode("test")); @@ -248,18 +258,33 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { function test_mint_slippage_revertAmount0() public { PositionConfig memory config = PositionConfig({poolKey: key, tickLower: -120, tickUpper: 120}); + uint256 liquidity = 1e18; + (uint256 amount0,) = LiquidityAmounts.getAmountsForLiquidity( + SQRT_PRICE_1_1, + TickMath.getSqrtPriceAtTick(config.tickLower), + TickMath.getSqrtPriceAtTick(config.tickUpper), + uint128(liquidity) + ); + bytes memory calls = - getMintEncoded(config, 1e18, 1 wei, MAX_SLIPPAGE_INCREASE, ActionConstants.MSG_SENDER, ZERO_BYTES); - vm.expectRevert(SlippageCheckLibrary.MaximumAmountExceeded.selector); + getMintEncoded(config, liquidity, 1 wei, MAX_SLIPPAGE_INCREASE, ActionConstants.MSG_SENDER, ZERO_BYTES); + vm.expectRevert(abi.encodeWithSelector(SlippageCheck.MaximumAmountExceeded.selector, 1 wei, amount0 + 1)); lpm.modifyLiquidities(calls, _deadline); } function test_mint_slippage_revertAmount1() public { PositionConfig memory config = PositionConfig({poolKey: key, tickLower: -120, tickUpper: 120}); + uint256 liquidity = 1e18; + (, uint256 amount1) = LiquidityAmounts.getAmountsForLiquidity( + SQRT_PRICE_1_1, + TickMath.getSqrtPriceAtTick(config.tickLower), + TickMath.getSqrtPriceAtTick(config.tickUpper), + uint128(liquidity) + ); bytes memory calls = - getMintEncoded(config, 1e18, MAX_SLIPPAGE_INCREASE, 1 wei, ActionConstants.MSG_SENDER, ZERO_BYTES); - vm.expectRevert(SlippageCheckLibrary.MaximumAmountExceeded.selector); + getMintEncoded(config, liquidity, MAX_SLIPPAGE_INCREASE, 1 wei, ActionConstants.MSG_SENDER, ZERO_BYTES); + vm.expectRevert(abi.encodeWithSelector(SlippageCheck.MaximumAmountExceeded.selector, 1 wei, amount1 + 1)); lpm.modifyLiquidities(calls, _deadline); } @@ -304,7 +329,9 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { // swap to move the price and cause a slippage revert swap(key, true, -1e18, ZERO_BYTES); - vm.expectRevert(SlippageCheckLibrary.MaximumAmountExceeded.selector); + vm.expectRevert( + abi.encodeWithSelector(SlippageCheck.MaximumAmountExceeded.selector, slippage, 1199947202932782783) + ); lpm.modifyLiquidities(calls, _deadline); } @@ -408,10 +435,12 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { uint256 tokenId = lpm.nextTokenId(); mint(config, 1e18, ActionConstants.MSG_SENDER, ZERO_BYTES); BalanceDelta delta = getLastDelta(); + uint128 amount0 = uint128(-delta.amount0()); - bytes memory calls = - getBurnEncoded(tokenId, config, uint128(-delta.amount0()) + 1 wei, MIN_SLIPPAGE_DECREASE, ZERO_BYTES); - vm.expectRevert(SlippageCheckLibrary.MinimumAmountInsufficient.selector); + bytes memory calls = getBurnEncoded(tokenId, config, amount0 + 1 wei, MIN_SLIPPAGE_DECREASE, ZERO_BYTES); + vm.expectRevert( + abi.encodeWithSelector(SlippageCheck.MinimumAmountInsufficient.selector, amount0 + 1, amount0 - 1) + ); lpm.modifyLiquidities(calls, _deadline); } @@ -420,10 +449,14 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { uint256 tokenId = lpm.nextTokenId(); mint(config, 1e18, ActionConstants.MSG_SENDER, ZERO_BYTES); BalanceDelta delta = getLastDelta(); + uint128 amount1 = uint128(-delta.amount1()); - bytes memory calls = - getBurnEncoded(tokenId, config, MIN_SLIPPAGE_DECREASE, uint128(-delta.amount1()) + 1 wei, ZERO_BYTES); - vm.expectRevert(SlippageCheckLibrary.MinimumAmountInsufficient.selector); + bytes memory calls = getBurnEncoded(tokenId, config, MIN_SLIPPAGE_DECREASE, amount1 + 1 wei, ZERO_BYTES); + + // reverts on amount1, because the swap sent token0 into the pool and took token1 + vm.expectRevert( + abi.encodeWithSelector(SlippageCheck.MinimumAmountInsufficient.selector, amount1 + 1, amount1 - 1) + ); lpm.modifyLiquidities(calls, _deadline); } @@ -450,15 +483,15 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { uint256 tokenId = lpm.nextTokenId(); mint(config, 1e18, ActionConstants.MSG_SENDER, ZERO_BYTES); BalanceDelta delta = getLastDelta(); + uint128 amount1 = uint128(-delta.amount1()); - bytes memory calls = getBurnEncoded( - tokenId, config, uint128(-delta.amount0()) - 1 wei, uint128(-delta.amount1()) - 1 wei, ZERO_BYTES - ); + bytes memory calls = + getBurnEncoded(tokenId, config, uint128(-delta.amount0()) - 1 wei, amount1 - 1 wei, ZERO_BYTES); // swap to move the price and cause a slippage revert swap(key, true, -1e18, ZERO_BYTES); - vm.expectRevert(SlippageCheckLibrary.MinimumAmountInsufficient.selector); + vm.expectRevert(abi.encodeWithSelector(SlippageCheck.MinimumAmountInsufficient.selector, amount1 - 1, 0)); lpm.modifyLiquidities(calls, _deadline); } @@ -610,11 +643,13 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { uint256 tokenId = lpm.nextTokenId(); mint(config, 1e18, ActionConstants.MSG_SENDER, ZERO_BYTES); BalanceDelta delta = getLastDelta(); + uint128 amount0Delta = uint128(-delta.amount0()); - bytes memory calls = getDecreaseEncoded( - tokenId, config, 1e18, uint128(-delta.amount0()) + 1 wei, MIN_SLIPPAGE_DECREASE, ZERO_BYTES + bytes memory calls = + getDecreaseEncoded(tokenId, config, 1e18, amount0Delta + 1, MIN_SLIPPAGE_DECREASE, ZERO_BYTES); + vm.expectRevert( + abi.encodeWithSelector(SlippageCheck.MinimumAmountInsufficient.selector, amount0Delta + 1, amount0Delta - 1) ); - vm.expectRevert(SlippageCheckLibrary.MinimumAmountInsufficient.selector); lpm.modifyLiquidities(calls, _deadline); } @@ -623,11 +658,13 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { uint256 tokenId = lpm.nextTokenId(); mint(config, 1e18, ActionConstants.MSG_SENDER, ZERO_BYTES); BalanceDelta delta = getLastDelta(); + uint128 amount1Delta = uint128(-delta.amount0()); - bytes memory calls = getDecreaseEncoded( - tokenId, config, 1e18, MIN_SLIPPAGE_DECREASE, uint128(-delta.amount1()) + 1 wei, ZERO_BYTES + bytes memory calls = + getDecreaseEncoded(tokenId, config, 1e18, MIN_SLIPPAGE_DECREASE, amount1Delta + 1 wei, ZERO_BYTES); + vm.expectRevert( + abi.encodeWithSelector(SlippageCheck.MinimumAmountInsufficient.selector, amount1Delta + 1, amount1Delta - 1) ); - vm.expectRevert(SlippageCheckLibrary.MinimumAmountInsufficient.selector); lpm.modifyLiquidities(calls, _deadline); } @@ -655,15 +692,16 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers { uint256 tokenId = lpm.nextTokenId(); mint(config, 1e18, ActionConstants.MSG_SENDER, ZERO_BYTES); BalanceDelta delta = getLastDelta(); + uint128 amount1 = uint128(-delta.amount1()); - bytes memory calls = getDecreaseEncoded( - tokenId, config, 1e18, uint128(-delta.amount0()) - 1 wei, uint128(-delta.amount1()) - 1 wei, ZERO_BYTES - ); + bytes memory calls = + getDecreaseEncoded(tokenId, config, 1e18, uint128(-delta.amount0()) - 1 wei, amount1 - 1 wei, ZERO_BYTES); // swap to move the price and cause a slippage revert swap(key, true, -1e18, ZERO_BYTES); - vm.expectRevert(SlippageCheckLibrary.MinimumAmountInsufficient.selector); + // reverts on amount1, because the swap sent token0 into the pool and took token1 + vm.expectRevert(abi.encodeWithSelector(SlippageCheck.MinimumAmountInsufficient.selector, amount1 - 1, 0)); lpm.modifyLiquidities(calls, _deadline); } diff --git a/test/router/Payments.t.sol b/test/router/Payments.t.sol index 3aa077efe..3b1b2c328 100644 --- a/test/router/Payments.t.sol +++ b/test/router/Payments.t.sol @@ -61,7 +61,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot { plan = plan.add(Actions.TAKE_ALL, abi.encode(key0.currency0, MIN_TAKE_AMOUNT)); bytes memory data = plan.encode(); - vm.expectRevert(IV4Router.V4TooMuchRequested.selector); + vm.expectRevert(abi.encodeWithSelector(IV4Router.V4TooMuchRequested.selector, amountIn - 1, amountIn)); router.executeActions(data); } @@ -73,10 +73,12 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot { plan = plan.add(Actions.SWAP_EXACT_IN_SINGLE, abi.encode(params)); plan = plan.add(Actions.SETTLE_ALL, abi.encode(key0.currency0, MAX_SETTLE_AMOUNT)); - plan = plan.add(Actions.TAKE_ALL, abi.encode(key0.currency0, expectedAmountOut + 1)); + plan = plan.add(Actions.TAKE_ALL, abi.encode(key0.currency1, expectedAmountOut + 1)); bytes memory data = plan.encode(); - vm.expectRevert(IV4Router.V4TooLittleReceived.selector); + vm.expectRevert( + abi.encodeWithSelector(IV4Router.V4TooLittleReceived.selector, expectedAmountOut + 1, expectedAmountOut) + ); router.executeActions(data); } @@ -92,7 +94,9 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot { plan = plan.add(Actions.TAKE_ALL, abi.encode(key0.currency0, MIN_TAKE_AMOUNT)); bytes memory data = plan.encode(); - vm.expectRevert(IV4Router.V4TooMuchRequested.selector); + vm.expectRevert( + abi.encodeWithSelector(IV4Router.V4TooMuchRequested.selector, expectedAmountIn - 1, expectedAmountIn) + ); router.executeActions(data); } @@ -105,10 +109,10 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot { plan = plan.add(Actions.SWAP_EXACT_OUT_SINGLE, abi.encode(params)); plan = plan.add(Actions.SETTLE_ALL, abi.encode(key0.currency0, MAX_SETTLE_AMOUNT)); - plan = plan.add(Actions.TAKE_ALL, abi.encode(key0.currency0, amountOut + 1)); + plan = plan.add(Actions.TAKE_ALL, abi.encode(key0.currency1, amountOut + 1)); bytes memory data = plan.encode(); - vm.expectRevert(IV4Router.V4TooLittleReceived.selector); + vm.expectRevert(abi.encodeWithSelector(IV4Router.V4TooLittleReceived.selector, amountOut + 1, amountOut)); router.executeActions(data); } @@ -121,10 +125,9 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot { plan = plan.add(Actions.SWAP_EXACT_OUT_SINGLE, abi.encode(params)); plan = plan.add(Actions.SETTLE_ALL, abi.encode(key0.currency0, expectedAmountIn)); - plan = plan.add(Actions.TAKE_ALL, abi.encode(key0.currency0, amountOut)); + plan = plan.add(Actions.TAKE_ALL, abi.encode(key0.currency1, amountOut)); bytes memory data = plan.encode(); - vm.expectRevert(IV4Router.V4TooLittleReceived.selector); router.executeActions(data); } diff --git a/test/router/V4Router.t.sol b/test/router/V4Router.t.sol index f6d63805e..db2316577 100644 --- a/test/router/V4Router.t.sol +++ b/test/router/V4Router.t.sol @@ -35,7 +35,9 @@ contract V4RouterTest is RoutingTestHelpers { plan = plan.add(Actions.SWAP_EXACT_IN_SINGLE, abi.encode(params)); bytes memory data = plan.finalizeSwap(key0.currency0, key0.currency1, ActionConstants.MSG_SENDER); - vm.expectRevert(IV4Router.V4TooLittleReceived.selector); + vm.expectRevert( + abi.encodeWithSelector(IV4Router.V4TooLittleReceived.selector, expectedAmountOut + 1, expectedAmountOut) + ); router.executeActions(data); } @@ -176,7 +178,9 @@ contract V4RouterTest is RoutingTestHelpers { plan = plan.add(Actions.SWAP_EXACT_IN, abi.encode(params)); bytes memory data = plan.finalizeSwap(key0.currency0, key0.currency1, ActionConstants.MSG_SENDER); - vm.expectRevert(IV4Router.V4TooLittleReceived.selector); + vm.expectRevert( + abi.encodeWithSelector(IV4Router.V4TooLittleReceived.selector, 992054607780215625 + 1, 992054607780215625) + ); router.executeActions(data); } @@ -484,7 +488,9 @@ contract V4RouterTest is RoutingTestHelpers { plan = plan.add(Actions.SWAP_EXACT_OUT_SINGLE, abi.encode(params)); bytes memory data = plan.finalizeSwap(key0.currency0, key0.currency1, ActionConstants.MSG_SENDER); - vm.expectRevert(IV4Router.V4TooMuchRequested.selector); + vm.expectRevert( + abi.encodeWithSelector(IV4Router.V4TooMuchRequested.selector, expectedAmountIn - 1, expectedAmountIn) + ); router.executeActions(data); } @@ -540,7 +546,9 @@ contract V4RouterTest is RoutingTestHelpers { plan = plan.add(Actions.SWAP_EXACT_OUT, abi.encode(params)); bytes memory data = plan.finalizeSwap(key0.currency0, key0.currency1, ActionConstants.MSG_SENDER); - vm.expectRevert(IV4Router.V4TooMuchRequested.selector); + vm.expectRevert( + abi.encodeWithSelector(IV4Router.V4TooMuchRequested.selector, expectedAmountIn - 1, expectedAmountIn) + ); router.executeActions(data); }