diff --git a/.forge-snapshots/PositionManager_burn_empty.snap b/.forge-snapshots/PositionManager_burn_empty.snap index 87a93dee8..770c5e1a7 100644 --- a/.forge-snapshots/PositionManager_burn_empty.snap +++ b/.forge-snapshots/PositionManager_burn_empty.snap @@ -1 +1 @@ -47359 \ No newline at end of file +47468 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_empty_native.snap b/.forge-snapshots/PositionManager_burn_empty_native.snap index 4a8a8af95..2096dba09 100644 --- a/.forge-snapshots/PositionManager_burn_empty_native.snap +++ b/.forge-snapshots/PositionManager_burn_empty_native.snap @@ -1 +1 @@ -47176 \ No newline at end of file +47286 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap index 825fac77b..25b88e960 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap @@ -1 +1 @@ -124007 \ No newline at end of file +124115 \ 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 6af4357b1..44d51e62b 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap @@ -1 +1 @@ -123514 \ No newline at end of file +123612 \ 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 d76dc05ab..345332e10 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap @@ -1 +1 @@ -131085 \ No newline at end of file +131193 \ 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 3c58f4718..f01928633 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap @@ -1 +1 @@ -130592 \ No newline at end of file +130691 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_native.snap b/.forge-snapshots/PositionManager_collect_native.snap index 88e501b80..43c80edae 100644 --- a/.forge-snapshots/PositionManager_collect_native.snap +++ b/.forge-snapshots/PositionManager_collect_native.snap @@ -1 +1 @@ -142673 \ No newline at end of file +142671 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_sameRange.snap b/.forge-snapshots/PositionManager_collect_sameRange.snap index 1a99484d7..2f02fb34f 100644 --- a/.forge-snapshots/PositionManager_collect_sameRange.snap +++ b/.forge-snapshots/PositionManager_collect_sameRange.snap @@ -1 +1 @@ -151521 \ No newline at end of file +151519 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_withClose.snap b/.forge-snapshots/PositionManager_collect_withClose.snap index 1a99484d7..2f02fb34f 100644 --- a/.forge-snapshots/PositionManager_collect_withClose.snap +++ b/.forge-snapshots/PositionManager_collect_withClose.snap @@ -1 +1 @@ -151521 \ No newline at end of file +151519 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_withTakePair.snap b/.forge-snapshots/PositionManager_collect_withTakePair.snap index 4bc49f33f..c025f4c1a 100644 --- a/.forge-snapshots/PositionManager_collect_withTakePair.snap +++ b/.forge-snapshots/PositionManager_collect_withTakePair.snap @@ -1 +1 @@ -150893 \ No newline at end of file +150879 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap index 28cb596ff..fa5a47aad 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap @@ -1 +1 @@ -109613 \ No newline at end of file +109612 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap index e933092b8..5e11e8b7a 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap @@ -1 +1 @@ -117064 \ No newline at end of file +117062 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap index 0073c6cd2..f7e0c5724 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap @@ -1 +1 @@ -116436 \ No newline at end of file +116422 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap index 51bb83b46..20bd32213 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap @@ -1 +1 @@ -135112 \ No newline at end of file +135220 \ 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 36235ed82..72f552d97 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap @@ -1 +1 @@ -127852 \ No newline at end of file +127960 \ 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 dabee1e42..bf2c52535 100644 --- a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap +++ b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap @@ -1 +1 @@ -129780 \ No newline at end of file +129778 \ 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 a82ce039c..5f3c88a05 100644 --- a/.forge-snapshots/PositionManager_decrease_take_take.snap +++ b/.forge-snapshots/PositionManager_decrease_take_take.snap @@ -1 +1 @@ -117597 \ No newline at end of file +117595 \ 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 b0d6ec681..c7fc0c590 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap @@ -1 +1 @@ -155771 \ No newline at end of file +155744 \ 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 6360928ab..88c9f5b81 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap @@ -1 +1 @@ -154773 \ No newline at end of file +154746 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap index 211696e53..ad4a5b7c1 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap @@ -1 +1 @@ -137571 \ No newline at end of file +137544 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap index b41afe105..2f980500e 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap @@ -1 +1 @@ -133916 \ No newline at end of file +133889 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap index 23f37561e..0d5e26c98 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap @@ -1 +1 @@ -174832 \ No newline at end of file +174805 \ 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 6022aba56..e8f34cd19 100644 --- a/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap +++ b/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap @@ -1 +1 @@ -144788 \ No newline at end of file +144761 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_native.snap b/.forge-snapshots/PositionManager_mint_native.snap index 13ca53795..9385d96d5 100644 --- a/.forge-snapshots/PositionManager_mint_native.snap +++ b/.forge-snapshots/PositionManager_mint_native.snap @@ -1 +1 @@ -341593 \ No newline at end of file +341266 \ 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 29480805e..75491bd15 100644 --- a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap +++ b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap @@ -1 +1 @@ -350085 \ No newline at end of file +349758 \ 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 fabd9615f..be445e63b 100644 --- a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap +++ b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap @@ -1 +1 @@ -349387 \ No newline at end of file +349060 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap index 62d4e212d..533290e7f 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap @@ -1 +1 @@ -319575 \ No newline at end of file +319248 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap index 4f1d17685..b9b0b83f8 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap @@ -1 +1 @@ -320217 \ No newline at end of file +319890 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_sameRange.snap b/.forge-snapshots/PositionManager_mint_sameRange.snap index 4b71527af..5918f3223 100644 --- a/.forge-snapshots/PositionManager_mint_sameRange.snap +++ b/.forge-snapshots/PositionManager_mint_sameRange.snap @@ -1 +1 @@ -245799 \ No newline at end of file +245472 \ 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 706f47dad..1fda57f12 100644 --- a/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap +++ b/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap @@ -1 +1 @@ -375617 \ No newline at end of file +375290 \ 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 0fa5f017c..9fdb5eb74 100644 --- a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap +++ b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap @@ -1 +1 @@ -325593 \ No newline at end of file +325266 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_withClose.snap b/.forge-snapshots/PositionManager_mint_withClose.snap index 16e498ae6..cadc3d86d 100644 --- a/.forge-snapshots/PositionManager_mint_withClose.snap +++ b/.forge-snapshots/PositionManager_mint_withClose.snap @@ -1 +1 @@ -376893 \ No newline at end of file +376566 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_withSettlePair.snap b/.forge-snapshots/PositionManager_mint_withSettlePair.snap index a1ad3e3a6..2e7d03aa4 100644 --- a/.forge-snapshots/PositionManager_mint_withSettlePair.snap +++ b/.forge-snapshots/PositionManager_mint_withSettlePair.snap @@ -1 +1 @@ -376033 \ No newline at end of file +375706 \ 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 64a1667cc..a2b68370c 100644 --- a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap +++ b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap @@ -1 +1 @@ -421320 \ No newline at end of file +420993 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_permit.snap b/.forge-snapshots/PositionManager_permit.snap index d3f21d676..d71fc9167 100644 --- a/.forge-snapshots/PositionManager_permit.snap +++ b/.forge-snapshots/PositionManager_permit.snap @@ -1 +1 @@ -79595 \ No newline at end of file +79470 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_permit_secondPosition.snap b/.forge-snapshots/PositionManager_permit_secondPosition.snap index 4bd3ebf9a..bf069bcae 100644 --- a/.forge-snapshots/PositionManager_permit_secondPosition.snap +++ b/.forge-snapshots/PositionManager_permit_secondPosition.snap @@ -1 +1 @@ -62483 \ No newline at end of file +62358 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_permit_twice.snap b/.forge-snapshots/PositionManager_permit_twice.snap index 39f8dac50..912820cbe 100644 --- a/.forge-snapshots/PositionManager_permit_twice.snap +++ b/.forge-snapshots/PositionManager_permit_twice.snap @@ -1 +1 @@ -45371 \ No newline at end of file +45246 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_subscribe.snap b/.forge-snapshots/PositionManager_subscribe.snap new file mode 100644 index 000000000..dec6506f4 --- /dev/null +++ b/.forge-snapshots/PositionManager_subscribe.snap @@ -0,0 +1 @@ +88474 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_unsubscribe.snap b/.forge-snapshots/PositionManager_unsubscribe.snap new file mode 100644 index 000000000..4efc0fb76 --- /dev/null +++ b/.forge-snapshots/PositionManager_unsubscribe.snap @@ -0,0 +1 @@ +62639 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_Bytecode.snap b/.forge-snapshots/V4Router_Bytecode.snap index 05c6c0881..bbc3f0f92 100644 --- a/.forge-snapshots/V4Router_Bytecode.snap +++ b/.forge-snapshots/V4Router_Bytecode.snap @@ -1 +1 @@ -8566 \ No newline at end of file +8773 \ No newline at end of file diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fb5820e5f..73de4cdbe 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -22,6 +22,7 @@ jobs: version: nightly - name: Run tests - run: forge test -vvv + run: forge test --isolate -vvv env: FOUNDRY_PROFILE: ci + FORGE_SNAPSHOT_CHECK: true diff --git a/script/DeployPosm.s.sol b/script/DeployPosm.s.sol index 92084bdca..3ce242504 100644 --- a/script/DeployPosm.s.sol +++ b/script/DeployPosm.s.sol @@ -12,10 +12,15 @@ import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol" contract DeployPosmTest is Script { function setUp() public {} - function run(address poolManager, address permit2) public returns (PositionManager posm) { + function run(address poolManager, address permit2, uint256 unsubscribeGasLimit) + public + returns (PositionManager posm) + { vm.startBroadcast(); - posm = new PositionManager{salt: hex"03"}(IPoolManager(poolManager), IAllowanceTransfer(permit2)); + posm = new PositionManager{salt: hex"03"}( + IPoolManager(poolManager), IAllowanceTransfer(permit2), unsubscribeGasLimit + ); console2.log("PositionManager", address(posm)); vm.stopBroadcast(); diff --git a/src/PositionManager.sol b/src/PositionManager.sol index 9aa0fade0..5bbeccd22 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.24; +pragma solidity 0.8.26; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.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 @@ -125,16 +125,17 @@ contract PositionManager is return positionConfigs[tokenId]; } - constructor(IPoolManager _poolManager, IAllowanceTransfer _permit2) + constructor(IPoolManager _poolManager, IAllowanceTransfer _permit2, uint256 _unsubscribeGasLimit) BaseActionsRouter(_poolManager) Permit2Forwarder(_permit2) ERC721Permit_v4("Uniswap V4 Positions NFT", "UNI-V4-POSM") + Notifier(_unsubscribeGasLimit) {} /// @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); _; } @@ -192,6 +193,7 @@ contract PositionManager is bytes calldata hookData ) = params.decodeModifyLiquidityParams(); _increase(tokenId, config, liquidity, amount0Max, amount1Max, hookData); + return; } else if (action == Actions.DECREASE_LIQUIDITY) { ( uint256 tokenId, @@ -202,6 +204,7 @@ contract PositionManager is bytes calldata hookData ) = params.decodeModifyLiquidityParams(); _decrease(tokenId, config, liquidity, amount0Min, amount1Min, hookData); + return; } else if (action == Actions.MINT_POSITION) { ( PositionConfig calldata config, @@ -212,6 +215,7 @@ contract PositionManager is bytes calldata hookData ) = params.decodeMintParams(); _mint(config, liquidity, amount0Max, amount1Max, _mapRecipient(owner), hookData); + return; } else if (action == Actions.BURN_POSITION) { // Will automatically decrease liquidity to 0 if the position is not already empty. ( @@ -222,35 +226,40 @@ contract PositionManager is bytes calldata hookData ) = params.decodeBurnParams(); _burn(tokenId, config, amount0Min, amount1Min, hookData); - } else { - revert UnsupportedAction(action); + return; } } else { if (action == Actions.SETTLE_PAIR) { (Currency currency0, Currency currency1) = params.decodeCurrencyPair(); _settlePair(currency0, currency1); + return; } else if (action == Actions.TAKE_PAIR) { - (Currency currency0, Currency currency1, address to) = params.decodeCurrencyPairAndAddress(); - _takePair(currency0, currency1, to); + (Currency currency0, Currency currency1, address recipient) = params.decodeCurrencyPairAndAddress(); + _takePair(currency0, currency1, _mapRecipient(recipient)); + return; } else if (action == Actions.SETTLE) { (Currency currency, uint256 amount, bool payerIsUser) = params.decodeCurrencyUint256AndBool(); _settle(currency, _mapPayer(payerIsUser), _mapSettleAmount(amount, currency)); + return; } else if (action == Actions.TAKE) { (Currency currency, address recipient, uint256 amount) = params.decodeCurrencyAddressAndUint256(); _take(currency, _mapRecipient(recipient), _mapTakeAmount(amount, currency)); + return; } else if (action == Actions.CLOSE_CURRENCY) { Currency currency = params.decodeCurrency(); _close(currency); + return; } else if (action == Actions.CLEAR_OR_TAKE) { (Currency currency, uint256 amountMax) = params.decodeCurrencyAndUint256(); _clearOrTake(currency, amountMax); + return; } else if (action == Actions.SWEEP) { (Currency currency, address to) = params.decodeCurrencyAndAddress(); _sweep(currency, _mapRecipient(to)); - } else { - revert UnsupportedAction(action); + return; } } + revert UnsupportedAction(action); } /// @dev Calling increase with 0 liquidity will credit the caller with any underlying fees of the position @@ -301,10 +310,9 @@ contract PositionManager is } _mint(owner, tokenId); - (BalanceDelta liquidityDelta, BalanceDelta feesAccrued) = - _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData); - // Slippage checks should be done on the principal liquidityDelta which is the liquidityDelta - feesAccrued - (liquidityDelta - feesAccrued).validateMaxIn(amount0Max, amount1Max); + // fee delta can be ignored as this is a new position + (BalanceDelta liquidityDelta,) = _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData); + liquidityDelta.validateMaxIn(amount0Max, amount1Max); positionConfigs[tokenId].setConfigId(config.toId()); emit MintPosition(tokenId, config); @@ -318,7 +326,7 @@ contract PositionManager is uint128 amount1Min, bytes calldata hookData ) internal onlyIfApproved(msgSender(), tokenId) onlyValidConfig(tokenId, config) { - uint256 liquidity = uint256(getPositionLiquidity(tokenId, config)); + uint256 liquidity = getPositionLiquidity(tokenId, config); // Can only call modify if there is non zero liquidity. if (liquidity > 0) { @@ -328,9 +336,13 @@ contract PositionManager is (liquidityDelta - feesAccrued).validateMinOut(amount0Min, amount1Min); } + bool hasSubscriber = positionConfigs[tokenId].hasSubscriber(); + delete positionConfigs[tokenId]; // Burn the token. _burn(tokenId); + + if (hasSubscriber) _unsubscribe(tokenId, config); } function _settlePair(Currency currency0, Currency currency1) internal { @@ -340,8 +352,7 @@ contract PositionManager is _settle(currency1, caller, _getFullDebt(currency1)); } - function _takePair(Currency currency0, Currency currency1, address to) internal { - address recipient = _mapRecipient(to); + function _takePair(Currency currency0, Currency currency1, address recipient) internal { _take(currency0, recipient, _getFullCredit(currency0)); _take(currency1, recipient, _getFullCredit(currency1)); } @@ -404,7 +415,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 7e0554d7d..f9faffed3 100644 --- a/src/V4Router.sol +++ b/src/V4Router.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.19; +pragma solidity 0.8.26; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; @@ -8,7 +8,7 @@ import {Currency} from "@uniswap/v4-core/src/types/Currency.sol"; import {TickMath} from "@uniswap/v4-core/src/libraries/TickMath.sol"; import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol"; -import {PathKey, PathKeyLib} from "./libraries/PathKey.sol"; +import {PathKey, PathKeyLibrary} from "./libraries/PathKey.sol"; import {CalldataDecoder} from "./libraries/CalldataDecoder.sol"; import {BipsLibrary} from "./libraries/BipsLibrary.sol"; import {IV4Router} from "./interfaces/IV4Router.sol"; @@ -25,7 +25,7 @@ import {ActionConstants} from "./libraries/ActionConstants.sol"; abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver { using SafeCastTemp for *; using SafeCast for *; - using PathKeyLib for PathKey; + using PathKeyLibrary for PathKey; using CalldataDecoder for bytes; using BipsLibrary for uint256; @@ -37,46 +37,53 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver { if (action == Actions.SWAP_EXACT_IN) { IV4Router.ExactInputParams calldata swapParams = params.decodeSwapExactInParams(); _swapExactInput(swapParams); + return; } else if (action == Actions.SWAP_EXACT_IN_SINGLE) { IV4Router.ExactInputSingleParams calldata swapParams = params.decodeSwapExactInSingleParams(); _swapExactInputSingle(swapParams); + return; } else if (action == Actions.SWAP_EXACT_OUT) { IV4Router.ExactOutputParams calldata swapParams = params.decodeSwapExactOutParams(); _swapExactOutput(swapParams); + return; } else if (action == Actions.SWAP_EXACT_OUT_SINGLE) { IV4Router.ExactOutputSingleParams calldata swapParams = params.decodeSwapExactOutSingleParams(); _swapExactOutputSingle(swapParams); - } else { - revert UnsupportedAction(action); + return; } } else { if (action == Actions.SETTLE_TAKE_PAIR) { (Currency settleCurrency, Currency takeCurrency) = params.decodeCurrencyPair(); _settle(settleCurrency, msgSender(), _getFullDebt(settleCurrency)); _take(takeCurrency, msgSender(), _getFullCredit(takeCurrency)); + return; } 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) { (Currency currency, uint256 amount, bool payerIsUser) = params.decodeCurrencyUint256AndBool(); _settle(currency, _mapPayer(payerIsUser), _mapSettleAmount(amount, currency)); + return; } else if (action == Actions.TAKE) { (Currency currency, address recipient, uint256 amount) = params.decodeCurrencyAddressAndUint256(); _take(currency, _mapRecipient(recipient), _mapTakeAmount(amount, currency)); + return; } else if (action == Actions.TAKE_PORTION) { (Currency currency, address recipient, uint256 bips) = params.decodeCurrencyAddressAndUint256(); _take(currency, _mapRecipient(recipient), _getFullCredit(currency).calculatePortion(bips)); - } else { - revert UnsupportedAction(action); + return; } } + revert UnsupportedAction(action); } function _swapExactInputSingle(IV4Router.ExactInputSingleParams calldata params) private { @@ -88,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 { @@ -111,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); } } @@ -125,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 { @@ -146,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/BaseActionsRouter.sol b/src/base/BaseActionsRouter.sol index 22abab016..56e311906 100644 --- a/src/base/BaseActionsRouter.sol +++ b/src/base/BaseActionsRouter.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-3.0-or-later -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; import {SafeCallback} from "./SafeCallback.sol"; @@ -40,7 +40,7 @@ abstract contract BaseActionsRouter is SafeCallback { if (numActions != params.length) revert InputLengthMismatch(); for (uint256 actionIndex = 0; actionIndex < numActions; actionIndex++) { - uint256 action = uint256(uint8(actions[actionIndex])); + uint256 action = uint8(actions[actionIndex]); _handleAction(action, params[actionIndex]); } diff --git a/src/base/DeltaResolver.sol b/src/base/DeltaResolver.sol index 801323c9b..a31bf1392 100644 --- a/src/base/DeltaResolver.sol +++ b/src/base/DeltaResolver.sol @@ -73,15 +73,17 @@ abstract contract DeltaResolver is ImmutableState { return currency.balanceOfSelf(); } else if (amount == ActionConstants.OPEN_DELTA) { return _getFullDebt(currency); + } else { + return amount; } - return amount; } /// @notice Calculates the amount for a take action function _mapTakeAmount(uint256 amount, Currency currency) internal view returns (uint256) { if (amount == ActionConstants.OPEN_DELTA) { return _getFullCredit(currency); + } else { + return amount; } - return amount; } } diff --git a/src/base/EIP712_v4.sol b/src/base/EIP712_v4.sol index 27366668f..e66261aa5 100644 --- a/src/base/EIP712_v4.sol +++ b/src/base/EIP712_v4.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; +pragma solidity ^0.8.0; import {IEIP712_v4} from "../interfaces/IEIP712_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)); @@ -26,7 +26,7 @@ contract EIP712_v4 is IEIP712_v4 { } /// @inheritdoc IEIP712_v4 - function DOMAIN_SEPARATOR() public view override returns (bytes32) { + function DOMAIN_SEPARATOR() public view returns (bytes32) { // uses cached version if chainid is unchanged from construction return block.chainid == _CACHED_CHAIN_ID ? _CACHED_DOMAIN_SEPARATOR : _buildDomainSeparator(); } diff --git a/src/base/ERC721Permit_v4.sol b/src/base/ERC721Permit_v4.sol index 3f03a03b4..b700db89d 100644 --- a/src/base/ERC721Permit_v4.sol +++ b/src/base/ERC721Permit_v4.sol @@ -1,9 +1,9 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; import {ERC721} from "solmate/src/tokens/ERC721.sol"; import {EIP712_v4} from "./EIP712_v4.sol"; -import {ERC721PermitHashLibrary} from "../libraries/ERC721PermitHash.sol"; +import {ERC721PermitHash} from "../libraries/ERC721PermitHash.sol"; import {SignatureVerification} from "permit2/src/libraries/SignatureVerification.sol"; import {IERC721Permit_v4} from "../interfaces/IERC721Permit_v4.sol"; @@ -17,6 +17,7 @@ abstract contract ERC721Permit_v4 is ERC721, IERC721Permit_v4, EIP712_v4, Unorde /// @notice Computes the nameHash and versionHash constructor(string memory name_, string memory symbol_) ERC721(name_, symbol_) EIP712_v4(name_) {} + /// @notice Checks if the block's timestamp is before a signature's deadline modifier checkSignatureDeadline(uint256 deadline) { if (block.timestamp > deadline) revert SignatureDeadlineExpired(); _; @@ -28,10 +29,10 @@ abstract contract ERC721Permit_v4 is ERC721, IERC721Permit_v4, EIP712_v4, Unorde payable checkSignatureDeadline(deadline) { - address owner = ownerOf(tokenId); - _checkNoSelfPermit(owner, spender); + // the .verify function checks the owner is non-0 + address owner = _ownerOf[tokenId]; - bytes32 digest = ERC721PermitHashLibrary.hashPermit(spender, tokenId, nonce, deadline); + bytes32 digest = ERC721PermitHash.hashPermit(spender, tokenId, nonce, deadline); signature.verify(_hashTypedData(digest), owner); _useUnorderedNonce(owner, nonce); @@ -47,9 +48,7 @@ abstract contract ERC721Permit_v4 is ERC721, IERC721Permit_v4, EIP712_v4, Unorde uint256 nonce, bytes calldata signature ) external payable checkSignatureDeadline(deadline) { - _checkNoSelfPermit(owner, operator); - - bytes32 digest = ERC721PermitHashLibrary.hashPermitForAll(operator, approved, nonce, deadline); + bytes32 digest = ERC721PermitHash.hashPermitForAll(operator, approved, nonce, deadline); signature.verify(_hashTypedData(digest), owner); _useUnorderedNonce(owner, nonce); @@ -74,7 +73,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 @@ -97,10 +96,6 @@ abstract contract ERC721Permit_v4 is ERC721, IERC721Permit_v4, EIP712_v4, Unorde || isApprovedForAll[ownerOf(tokenId)][spender]; } - function _checkNoSelfPermit(address owner, address permitted) internal pure { - if (owner == permitted) revert NoSelfPermit(); - } - // TODO: to be implemented after audits function tokenURI(uint256) public pure override returns (string memory) { return "https://example.com"; diff --git a/src/base/ImmutableState.sol b/src/base/ImmutableState.sol index dab1563cc..8e30f1a4b 100644 --- a/src/base/ImmutableState.sol +++ b/src/base/ImmutableState.sol @@ -1,9 +1,12 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.19; +pragma solidity ^0.8.0; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; +/// @title Immutable State +/// @notice A collection of immutable state variables, commonly used across multiple contracts contract ImmutableState { + /// @notice The Uniswap v4 PoolManager contract IPoolManager public immutable poolManager; constructor(IPoolManager _poolManager) { diff --git a/src/base/Multicall_v4.sol b/src/base/Multicall_v4.sol index e648cca8d..e632270af 100644 --- a/src/base/Multicall_v4.sol +++ b/src/base/Multicall_v4.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.19; +pragma solidity ^0.8.0; import {IMulticall_v4} from "../interfaces/IMulticall_v4.sol"; @@ -7,7 +7,7 @@ import {IMulticall_v4} from "../interfaces/IMulticall_v4.sol"; /// @notice Enables calling multiple methods in a single call to the contract abstract contract Multicall_v4 is IMulticall_v4 { /// @inheritdoc IMulticall_v4 - function multicall(bytes[] calldata data) external payable override returns (bytes[] memory results) { + function multicall(bytes[] calldata data) external payable returns (bytes[] memory results) { results = new bytes[](data.length); for (uint256 i = 0; i < data.length; i++) { (bool success, bytes memory result) = address(this).delegatecall(data[i]); diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol index b7f3652db..bb94f73a7 100644 --- a/src/base/Notifier.sol +++ b/src/base/Notifier.sol @@ -1,36 +1,40 @@ // SPDX-License-Identifier: GPL-3.0-or-later -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; import {ISubscriber} from "../interfaces/ISubscriber.sol"; import {PositionConfig} from "../libraries/PositionConfig.sol"; import {PositionConfigId, PositionConfigIdLibrary} from "../libraries/PositionConfigId.sol"; -import {BipsLibrary} from "../libraries/BipsLibrary.sol"; import {INotifier} from "../interfaces/INotifier.sol"; import {CustomRevert} from "@uniswap/v4-core/src/libraries/CustomRevert.sol"; import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; /// @notice Notifier is used to opt in to sending updates to external contracts about position modifications or transfers abstract contract Notifier is INotifier { - using BipsLibrary for uint256; 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 - uint256 private constant BLOCK_LIMIT_BPS = 100; + /// @inheritdoc INotifier + uint256 public immutable unsubscribeGasLimit; /// @inheritdoc INotifier mapping(uint256 tokenId => ISubscriber subscriber) public subscriber; + constructor(uint256 _unsubscribeGasLimit) { + unsubscribeGasLimit = _unsubscribeGasLimit; + } + + /// @notice Only allow callers that are approved as spenders or operators of the tokenId + /// @dev to be implemented by the parent contract (PositionManager) + /// @param caller the address of the caller + /// @param tokenId the tokenId of the position modifier onlyIfApproved(address caller, uint256 tokenId) virtual; + + /// @notice Only allow callers that provide the correct config for the tokenId + /// @dev to be implemented by the parent contract (PositionManager) + /// @param tokenId the tokenId of the position + /// @param config the config of the tokenId modifier onlyValidConfig(uint256 tokenId, PositionConfig calldata config) virtual; function _positionConfigs(uint256 tokenId) internal view virtual returns (PositionConfigId storage); @@ -46,7 +50,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,25 +59,35 @@ abstract contract Notifier is INotifier { Wrap__SubscriptionReverted.selector.bubbleUpAndRevertWith(newSubscriber); } - emit Subscribed(tokenId, newSubscriber); + emit Subscription(tokenId, newSubscriber); } /// @inheritdoc INotifier - function unsubscribe(uint256 tokenId, PositionConfig calldata config, bytes calldata data) + function unsubscribe(uint256 tokenId, PositionConfig calldata config) external payable onlyIfApproved(msg.sender, tokenId) onlyValidConfig(tokenId, config) { + if (!_positionConfigs(tokenId).hasSubscriber()) NotSubscribed.selector.revertWith(); + _unsubscribe(tokenId, config); + } + + function _unsubscribe(uint256 tokenId, PositionConfig calldata config) internal { _positionConfigs(tokenId).setUnsubscribe(); ISubscriber _subscriber = subscriber[tokenId]; delete subscriber[tokenId]; - uint256 subscriberGasLimit = block.gaslimit.calculatePortion(BLOCK_LIMIT_BPS); - try _subscriber.notifyUnsubscribe{gas: subscriberGasLimit}(tokenId, config, data) {} catch {} + if (address(_subscriber).code.length > 0) { + // require that the remaining gas is sufficient to notify the subscriber + // otherwise, users can select a gas limit where .notifyUnsubscribe hits OutOfGas yet the + // transaction/unsubscription can still succeed + if (gasleft() < unsubscribeGasLimit) GasLimitTooLow.selector.revertWith(); + try _subscriber.notifyUnsubscribe{gas: unsubscribeGasLimit}(tokenId, config) {} catch {} + } - emit Unsubscribed(tokenId, address(_subscriber)); + emit Unsubscription(tokenId, address(_subscriber)); } function _notifyModifyLiquidity( @@ -106,6 +120,7 @@ abstract contract Notifier is INotifier { } function _call(address target, bytes memory encodedCall) internal returns (bool success) { + if (target.code.length == 0) NoCodeSubscriber.selector.revertWith(); assembly ("memory-safe") { success := call(gas(), target, 0, add(encodedCall, 0x20), mload(encodedCall), 0, 0) } diff --git a/src/base/Permit2Forwarder.sol b/src/base/Permit2Forwarder.sol index 41525c6e3..b7c678683 100644 --- a/src/base/Permit2Forwarder.sol +++ b/src/base/Permit2Forwarder.sol @@ -1,32 +1,51 @@ // SPDX-License-Identifier: GPL-3.0-or-later -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol"; /// @notice PermitForwarder allows permitting this contract as a spender on permit2 /// @dev This contract does not enforce the spender to be this contract, but that is the intended use case contract Permit2Forwarder { + /// @notice the Permit2 contract to forward approvals IAllowanceTransfer public immutable permit2; + error Wrap__Permit2Reverted(address _permit2, bytes reason); + constructor(IAllowanceTransfer _permit2) { permit2 = _permit2; } /// @notice allows forwarding a single permit to permit2 /// @dev this function is payable to allow multicall with NATIVE based actions + /// @param owner the owner of the tokens + /// @param permitSingle the permit data + /// @param signature the signature of the permit; abi.encodePacked(r, s, v) 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 /// @dev this function is payable to allow multicall with NATIVE based actions + /// @param owner the owner of the tokens + /// @param _permitBatch a batch of approvals + /// @param signature the signature of the permit; abi.encodePacked(r, s, v) 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/PoolInitializer.sol b/src/base/PoolInitializer.sol index 09059f513..1f42ab1ff 100644 --- a/src/base/PoolInitializer.sol +++ b/src/base/PoolInitializer.sol @@ -1,11 +1,18 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; import {ImmutableState} from "./ImmutableState.sol"; import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; +/// @title Pool Initializer +/// @notice Initializes a Uniswap v4 Pool +/// @dev Enables create pool + mint liquidity in a single transaction with multicall abstract contract PoolInitializer is ImmutableState { + /// @notice Initialize a Uniswap v4 Pool + /// @param key the PoolKey of the pool to initialize + /// @param sqrtPriceX96 the initial sqrtPriceX96 of the pool + /// @param hookData the optional data passed to the hook's initialize functions function initializePool(PoolKey calldata key, uint160 sqrtPriceX96, bytes calldata hookData) external payable diff --git a/src/base/SafeCallback.sol b/src/base/SafeCallback.sol index 99942a9e6..cda2b4a04 100644 --- a/src/base/SafeCallback.sol +++ b/src/base/SafeCallback.sol @@ -1,24 +1,30 @@ // SPDX-License-Identifier: GPL-3.0-or-later -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; import {IUnlockCallback} from "@uniswap/v4-core/src/interfaces/callback/IUnlockCallback.sol"; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; import {ImmutableState} from "./ImmutableState.sol"; +/// @title Safe Callback +/// @notice A contract that only allows the Uniswap v4 PoolManager to call the unlockCallback abstract contract SafeCallback is ImmutableState, IUnlockCallback { + /// @notice Thrown when calling unlockCallback where the caller is not PoolManager error NotPoolManager(); constructor(IPoolManager _poolManager) ImmutableState(_poolManager) {} - modifier onlyByPoolManager() { + /// @notice Only allow calls from the PoolManager contract + 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) { + /// @inheritdoc IUnlockCallback + /// @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); } + /// @dev to be implemented by the child contract, to safely guarantee the logic is only executed by the PoolManager function _unlockCallback(bytes calldata data) internal virtual returns (bytes memory); } diff --git a/src/base/UnorderedNonce.sol b/src/base/UnorderedNonce.sol index 13feb1081..fc33b63d8 100644 --- a/src/base/UnorderedNonce.sol +++ b/src/base/UnorderedNonce.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; /// @title Unordered Nonce /// @notice Contract state and methods for using unordered nonces in signatures diff --git a/src/base/hooks/BaseHook.sol b/src/base/hooks/BaseHook.sol index 0c983cf6e..331d513c6 100644 --- a/src/base/hooks/BaseHook.sol +++ b/src/base/hooks/BaseHook.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-3.0-or-later -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; @@ -9,6 +9,8 @@ import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; import {BeforeSwapDelta} from "@uniswap/v4-core/src/types/BeforeSwapDelta.sol"; import {SafeCallback} from "../SafeCallback.sol"; +/// @title Base Hook +/// @notice abstract contract for hook implementations abstract contract BaseHook is IHooks, SafeCallback { error NotSelf(); error InvalidPool(); @@ -31,11 +33,14 @@ abstract contract BaseHook is IHooks, SafeCallback { _; } + /// @notice Returns a struct of permissions to signal which hook functions are to be implemented + /// @dev Used at deployment to validate the address correctly represents the expected permissions function getHookPermissions() public pure virtual returns (Hooks.Permissions memory); - // this function is virtual so that we can override it during testing, - // which allows us to deploy an implementation to any address - // and then etch the bytecode into the correct address + /// @notice Validates the deployed hook address agrees with the expected permissions of the hook + /// @dev this function is virtual so that we can override it during testing, + /// which allows us to deploy an implementation to any address + /// and then etch the bytecode into the correct address function validateHookAddress(BaseHook _this) internal pure virtual { Hooks.validateHookPermissions(_this, getHookPermissions()); } @@ -45,16 +50,17 @@ abstract contract BaseHook is IHooks, SafeCallback { if (success) return returnData; if (returnData.length == 0) revert LockFailure(); // if the call failed, bubble up the reason - /// @solidity memory-safe-assembly - assembly { + assembly ("memory-safe") { revert(add(returnData, 32), mload(returnData)) } } + /// @inheritdoc IHooks function beforeInitialize(address, PoolKey calldata, uint160, bytes calldata) external virtual returns (bytes4) { revert HookNotImplemented(); } + /// @inheritdoc IHooks function afterInitialize(address, PoolKey calldata, uint160, int24, bytes calldata) external virtual @@ -63,6 +69,7 @@ abstract contract BaseHook is IHooks, SafeCallback { revert HookNotImplemented(); } + /// @inheritdoc IHooks function beforeAddLiquidity(address, PoolKey calldata, IPoolManager.ModifyLiquidityParams calldata, bytes calldata) external virtual @@ -71,6 +78,7 @@ abstract contract BaseHook is IHooks, SafeCallback { revert HookNotImplemented(); } + /// @inheritdoc IHooks function beforeRemoveLiquidity( address, PoolKey calldata, @@ -80,6 +88,7 @@ abstract contract BaseHook is IHooks, SafeCallback { revert HookNotImplemented(); } + /// @inheritdoc IHooks function afterAddLiquidity( address, PoolKey calldata, @@ -90,6 +99,7 @@ abstract contract BaseHook is IHooks, SafeCallback { revert HookNotImplemented(); } + /// @inheritdoc IHooks function afterRemoveLiquidity( address, PoolKey calldata, @@ -100,6 +110,7 @@ abstract contract BaseHook is IHooks, SafeCallback { revert HookNotImplemented(); } + /// @inheritdoc IHooks function beforeSwap(address, PoolKey calldata, IPoolManager.SwapParams calldata, bytes calldata) external virtual @@ -108,6 +119,7 @@ abstract contract BaseHook is IHooks, SafeCallback { revert HookNotImplemented(); } + /// @inheritdoc IHooks function afterSwap(address, PoolKey calldata, IPoolManager.SwapParams calldata, BalanceDelta, bytes calldata) external virtual @@ -116,6 +128,7 @@ abstract contract BaseHook is IHooks, SafeCallback { revert HookNotImplemented(); } + /// @inheritdoc IHooks function beforeDonate(address, PoolKey calldata, uint256, uint256, bytes calldata) external virtual @@ -124,6 +137,7 @@ abstract contract BaseHook is IHooks, SafeCallback { revert HookNotImplemented(); } + /// @inheritdoc IHooks function afterDonate(address, PoolKey calldata, uint256, uint256, bytes calldata) external virtual diff --git a/src/interfaces/IERC721Permit_v4.sol b/src/interfaces/IERC721Permit_v4.sol index 637e9b33f..bc4c7aa06 100644 --- a/src/interfaces/IERC721Permit_v4.sol +++ b/src/interfaces/IERC721Permit_v4.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity >=0.7.5; +pragma solidity ^0.8.0; /// @title ERC721 with permit /// @notice Extension to ERC721 that includes a permit function for signature based approvals @@ -12,6 +12,7 @@ interface IERC721Permit_v4 { /// @param spender The account that is being approved /// @param tokenId The ID of the token that is being approved for spending /// @param deadline The deadline timestamp by which the call must be mined for the approve to work + /// @param nonce a unique value, for an owner, to prevent replay attacks; an unordered nonce where the top 248 bits correspond to a word and the bottom 8 bits calculate the bit position of the word /// @param signature Concatenated data from a valid secp256k1 signature from the holder, i.e. abi.encodePacked(r, s, v) /// @dev payable so it can be multicalled with NATIVE related actions function permit(address spender, uint256 tokenId, uint256 deadline, uint256 nonce, bytes calldata signature) @@ -23,7 +24,7 @@ interface IERC721Permit_v4 { /// @param operator The address that will be set as an operator for the owner /// @param approved The permission to set on the operator /// @param deadline The deadline timestamp by which the call must be mined for the approve to work - /// @param nonce The nonce of the owner's permit + /// @param nonce a unique value, for an owner, to prevent replay attacks; an unordered nonce where the top 248 bits correspond to a word and the bottom 8 bits calculate the bit position of the word /// @param signature Concatenated data from a valid secp256k1 signature from the holder, i.e. abi.encodePacked(r, s, v) /// @dev payable so it can be multicalled with NATIVE related actions function permitForAll( diff --git a/src/interfaces/IMulticall_v4.sol b/src/interfaces/IMulticall_v4.sol index f1629f129..1d053a97d 100644 --- a/src/interfaces/IMulticall_v4.sol +++ b/src/interfaces/IMulticall_v4.sol @@ -1,11 +1,12 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.19; +pragma solidity ^0.8.0; /// @title Multicall_v4 interface /// @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 e721d259f..17bb8f961 100644 --- a/src/interfaces/INotifier.sol +++ b/src/interfaces/INotifier.sol @@ -1,17 +1,30 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; import {PositionConfig} from "../libraries/PositionConfig.sol"; import {ISubscriber} from "./ISubscriber.sol"; /// @notice This interface is used to opt in to sending updates to external contracts about position modifications or transfers interface INotifier { + /// @notice Thrown when unsubscribing without a subscriber + error NotSubscribed(); + /// @notice Thrown when a subscriber does not have code + error NoCodeSubscriber(); + /// @notice Thrown when a user specifies a gas limit too low to avoid valid unsubscribe notifications + error GasLimitTooLow(); /// @notice Wraps the revert message of the subscriber contract on a reverting subscription error Wrap__SubscriptionReverted(address subscriber, bytes reason); /// @notice Wraps the revert message of the subscriber contract on a reverting modify liquidity notification 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 @@ -32,13 +45,17 @@ interface INotifier { /// @notice Removes the subscriber from receiving notifications for a respective position /// @param tokenId the ERC721 tokenId /// @param config the corresponding PositionConfig for the tokenId - /// @param data caller-provided data that's forwarded to the subscriber contract + /// @dev Callers must specify a high gas limit (remaining gas should be higher than subscriberGasLimit) such that the subscriber can be notified /// @dev payable so it can be multicalled with NATIVE related actions /// @dev Must always allow a user to unsubscribe. In the case of a malicious subscriber, a user can always unsubscribe safely, ensuring liquidity is always modifiable. - function unsubscribe(uint256 tokenId, PositionConfig calldata config, bytes calldata data) external payable; + function unsubscribe(uint256 tokenId, PositionConfig calldata config) external payable; /// @notice Returns whether a position should call out to notify a subscribing contract on modification or transfer /// @param tokenId the ERC721 tokenId /// @return bool whether or not the position has a subscriber function hasSubscriber(uint256 tokenId) external view returns (bool); + + /// @notice Returns and determines the maximum allowable gas-used for notifying unsubscribe + /// @return uint256 the maximum gas limit when notifying a subscriber's `notifyUnsubscribe` function + function unsubscribeGasLimit() external view returns (uint256); } diff --git a/src/interfaces/IPositionManager.sol b/src/interfaces/IPositionManager.sol index 689146b0a..ff0e03fde 100644 --- a/src/interfaces/IPositionManager.sol +++ b/src/interfaces/IPositionManager.sol @@ -1,15 +1,21 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; import {PositionConfig} from "../libraries/PositionConfig.sol"; import {INotifier} from "./INotifier.sol"; +/// @title IPositionManager +/// @notice Interface for the PositionManager contract interface IPositionManager is INotifier { + /// @notice Thrown when the caller is not approved to modify a position error NotApproved(address caller); - error DeadlinePassed(); + /// @notice Thrown when the block.timestamp exceeds the user-provided deadline + error DeadlinePassed(uint256 deadline); + /// @notice Thrown when the caller provides the incorrect PositionConfig for a corresponding tokenId when modifying liquidity error IncorrectPositionConfigForTokenId(uint256 tokenId); + /// @notice Emitted when a new liquidity position is minted event MintPosition(uint256 indexed tokenId, PositionConfig config); /// @notice Unlocks Uniswap v4 PoolManager and batches actions for modifying liquidity diff --git a/src/interfaces/IQuoter.sol b/src/interfaces/IQuoter.sol index 57db3778c..8f113f4c6 100644 --- a/src/interfaces/IQuoter.sol +++ b/src/interfaces/IQuoter.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.20; +pragma solidity ^0.8.0; import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; import {Currency} from "@uniswap/v4-core/src/types/Currency.sol"; diff --git a/src/interfaces/ISubscriber.sol b/src/interfaces/ISubscriber.sol index 18d428c7b..a1393c7a9 100644 --- a/src/interfaces/ISubscriber.sol +++ b/src/interfaces/ISubscriber.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; import {PositionConfig} from "../libraries/PositionConfig.sol"; import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; @@ -10,10 +10,12 @@ interface ISubscriber { /// @param config details about the position /// @param data additional data passed in by the caller function notifySubscribe(uint256 tokenId, PositionConfig memory config, bytes memory data) external; + /// @notice Called when a position unsubscribes from the subscriber + /// @dev This call's gas is capped at `unsubscribeGasLimit` (set at deployment) + /// @dev Because of EIP-150, solidity may only allocate 63/64 of gasleft() /// @param tokenId the token ID of the position /// @param config details about the position - /// @param data additional data passed in by the caller - function notifyUnsubscribe(uint256 tokenId, PositionConfig memory config, bytes memory data) external; + function notifyUnsubscribe(uint256 tokenId, PositionConfig memory config) external; /// @param tokenId the token ID of the position /// @param config details about the position /// @param liquidityChange the change in liquidity on the underlying position diff --git a/src/interfaces/IV4Router.sol b/src/interfaces/IV4Router.sol index 8bde55bf4..13ed4775f 100644 --- a/src/interfaces/IV4Router.sol +++ b/src/interfaces/IV4Router.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.19; +pragma solidity ^0.8.0; import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; import {Currency} from "@uniswap/v4-core/src/types/Currency.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/interfaces/external/IERC20PermitAllowed.sol b/src/interfaces/external/IERC20PermitAllowed.sol deleted file mode 100644 index 7f2cf6570..000000000 --- a/src/interfaces/external/IERC20PermitAllowed.sol +++ /dev/null @@ -1,27 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity >=0.5.0; - -/// @title Interface for permit -/// @notice Interface used by DAI/CHAI for permit -interface IERC20PermitAllowed { - /// @notice Approve the spender to spend some tokens via the holder signature - /// @dev This is the permit interface used by DAI and CHAI - /// @param holder The address of the token holder, the token owner - /// @param spender The address of the token spender - /// @param nonce The holder's nonce, increases at each call to permit - /// @param expiry The timestamp at which the permit is no longer valid - /// @param allowed Boolean that sets approval amount, true for type(uint256).max and false for 0 - /// @param v Must produce valid secp256k1 signature from the holder along with `r` and `s` - /// @param r Must produce valid secp256k1 signature from the holder along with `v` and `s` - /// @param s Must produce valid secp256k1 signature from the holder along with `r` and `v` - function permit( - address holder, - address spender, - uint256 nonce, - uint256 expiry, - bool allowed, - uint8 v, - bytes32 r, - bytes32 s - ) external; -} diff --git a/src/lens/Quoter.sol b/src/lens/Quoter.sol index 17443b6b2..4f9741943 100644 --- a/src/lens/Quoter.sol +++ b/src/lens/Quoter.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.20; +pragma solidity ^0.8.0; import {TickMath} from "@uniswap/v4-core/src/libraries/TickMath.sol"; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; @@ -9,13 +9,13 @@ import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; import {PoolIdLibrary} from "@uniswap/v4-core/src/types/PoolId.sol"; import {IQuoter} from "../interfaces/IQuoter.sol"; import {PoolTicksCounter} from "../libraries/PoolTicksCounter.sol"; -import {PathKey, PathKeyLib} from "../libraries/PathKey.sol"; +import {PathKey, PathKeyLibrary} from "../libraries/PathKey.sol"; import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; import {SafeCallback} from "../base/SafeCallback.sol"; contract Quoter is IQuoter, SafeCallback { using PoolIdLibrary for PoolKey; - using PathKeyLib for PathKey; + using PathKeyLibrary for PathKey; using StateLibrary for IPoolManager; /// @dev cache used to check a safety condition in exact output swaps. @@ -54,7 +54,6 @@ contract Quoter is IQuoter, SafeCallback { /// @inheritdoc IQuoter function quoteExactInputSingle(QuoteExactSingleParams memory params) public - override returns (int128[] memory deltaAmounts, uint160 sqrtPriceX96After, uint32 initializedTicksLoaded) { try poolManager.unlock(abi.encodeCall(this._quoteExactInputSingle, (params))) {} @@ -81,7 +80,6 @@ contract Quoter is IQuoter, SafeCallback { /// @inheritdoc IQuoter function quoteExactOutputSingle(QuoteExactSingleParams memory params) public - override returns (int128[] memory deltaAmounts, uint160 sqrtPriceX96After, uint32 initializedTicksLoaded) { try poolManager.unlock(abi.encodeCall(this._quoteExactOutputSingle, (params))) {} @@ -94,7 +92,6 @@ contract Quoter is IQuoter, SafeCallback { /// @inheritdoc IQuoter function quoteExactOutput(QuoteExactParams memory params) public - override returns ( int128[] memory deltaAmounts, uint160[] memory sqrtPriceX96AfterList, @@ -112,8 +109,7 @@ contract Quoter is IQuoter, SafeCallback { if (success) return returnData; if (returnData.length == 0) revert LockFailure(); // if the call failed, bubble up the reason - /// @solidity memory-safe-assembly - assembly { + assembly ("memory-safe") { revert(add(returnData, 32), mload(returnData)) } } @@ -235,7 +231,7 @@ contract Quoter is IQuoter, SafeCallback { curAmountOut = i == pathLength ? params.exactAmount : cache.prevAmount; amountOutCached = curAmountOut; - (PoolKey memory poolKey, bool oneForZero) = PathKeyLib.getPoolAndSwapDirection( + (PoolKey memory poolKey, bool oneForZero) = PathKeyLibrary.getPoolAndSwapDirection( params.path[i - 1], i == pathLength ? params.exactCurrency : cache.prevCurrency ); diff --git a/src/lens/StateView.sol b/src/lens/StateView.sol index abee97bdc..f361b905a 100644 --- a/src/lens/StateView.sol +++ b/src/lens/StateView.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.20; +pragma solidity ^0.8.0; import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; import {PoolId} from "@uniswap/v4-core/src/types/PoolId.sol"; diff --git a/src/libraries/ActionConstants.sol b/src/libraries/ActionConstants.sol index e371f5496..914528b02 100644 --- a/src/libraries/ActionConstants.sol +++ b/src/libraries/ActionConstants.sol @@ -1,6 +1,9 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.19; +pragma solidity ^0.8.0; +/// @title Action Constants +/// @notice Common constants used in actions +/// @dev Constants are gas efficient alternatives to their literal values library ActionConstants { /// @notice used to signal that an action should use the input value of the open delta on the pool manager /// or of the balance that the contract holds @@ -9,7 +12,9 @@ library ActionConstants { /// This value is equivalent to 1<<255, i.e. a singular 1 in the most significant bit. uint256 internal constant CONTRACT_BALANCE = 0x8000000000000000000000000000000000000000000000000000000000000000; - /// @notice used to signal that the recipient of an action should be the msgSender or address(this) + /// @notice used to signal that the recipient of an action should be the msgSender address internal constant MSG_SENDER = address(1); + + /// @notice used to signal that the recipient of an action should be the address(this) address internal constant ADDRESS_THIS = address(2); } diff --git a/src/libraries/Actions.sol b/src/libraries/Actions.sol index 0b2592dd6..49d3e04f1 100644 --- a/src/libraries/Actions.sol +++ b/src/libraries/Actions.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-3.0-or-later -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; /// @notice Library to define different pool actions. /// @dev These are suggested common commands, however additional commands should be defined as required diff --git a/src/libraries/BipsLibrary.sol b/src/libraries/BipsLibrary.sol index 32bd97fc8..f5a842fa3 100644 --- a/src/libraries/BipsLibrary.sol +++ b/src/libraries/BipsLibrary.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.19; +pragma solidity ^0.8.0; /// @title For calculating a percentage of an amount, using bips // TODO: Post-audit move to core, as v4-core will use something similar. diff --git a/src/libraries/ERC721PermitHash.sol b/src/libraries/ERC721PermitHash.sol index 83fe06f73..a8c9cfc87 100644 --- a/src/libraries/ERC721PermitHash.sol +++ b/src/libraries/ERC721PermitHash.sol @@ -1,13 +1,19 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; -library ERC721PermitHashLibrary { +library ERC721PermitHash { /// @dev Value is equal to keccak256("Permit(address spender,uint256 tokenId,uint256 nonce,uint256 deadline)"); bytes32 constant PERMIT_TYPEHASH = 0x49ecf333e5b8c95c40fdafc95c1ad136e8914a8fb55e9dc8bb01eaa83a2df9ad; /// @dev Value is equal to keccak256("PermitForAll(address operator,bool approved,uint256 nonce,uint256 deadline)"); bytes32 constant PERMIT_FOR_ALL_TYPEHASH = 0x6673cb397ee2a50b6b8401653d3638b4ac8b3db9c28aa6870ffceb7574ec2f76; + /// @notice Hashes the data that will be signed for IERC721Permit_v4.permit() + /// @param spender The address which may spend the tokenId + /// @param tokenId The tokenId of the owner, which may be spent by spender + /// @param nonce A unique non-ordered value for each signature to prevent replay attacks + /// @param deadline The time at which the signature expires + /// @return digest The hash of the data to be signed; the equivalent to keccak256(abi.encode(PERMIT_TYPEHASH, spender, tokenId, nonce, deadline)); function hashPermit(address spender, uint256 tokenId, uint256 nonce, uint256 deadline) internal pure @@ -17,7 +23,7 @@ library ERC721PermitHashLibrary { assembly ("memory-safe") { let fmp := mload(0x40) mstore(fmp, PERMIT_TYPEHASH) - mstore(add(fmp, 0x20), spender) + mstore(add(fmp, 0x20), and(spender, 0xffffffffffffffffffffffffffffffffffffffff)) mstore(add(fmp, 0x40), tokenId) mstore(add(fmp, 0x60), nonce) mstore(add(fmp, 0x80), deadline) @@ -32,6 +38,12 @@ library ERC721PermitHashLibrary { } } + /// @notice Hashes the data that will be signed for IERC721Permit_v4.permit() + /// @param operator The address which may spend any of the owner's tokenIds + /// @param approved true if the operator is to have full permission over the owner's tokenIds; false otherwise + /// @param nonce A unique non-ordered value for each signature to prevent replay attacks + /// @param deadline The time at which the signature expires + /// @return digest The hash of the data to be signed; the equivalent to keccak256(abi.encode(PERMIT_FOR_ALL_TYPEHASH, operator, approved, nonce, deadline)); function hashPermitForAll(address operator, bool approved, uint256 nonce, uint256 deadline) internal pure @@ -41,8 +53,8 @@ library ERC721PermitHashLibrary { assembly ("memory-safe") { let fmp := mload(0x40) mstore(fmp, PERMIT_FOR_ALL_TYPEHASH) - mstore(add(fmp, 0x20), operator) - mstore(add(fmp, 0x40), approved) + mstore(add(fmp, 0x20), and(operator, 0xffffffffffffffffffffffffffffffffffffffff)) + mstore(add(fmp, 0x40), and(approved, 0x1)) mstore(add(fmp, 0x60), nonce) mstore(add(fmp, 0x80), deadline) digest := keccak256(fmp, 0xa0) diff --git a/src/libraries/PathKey.sol b/src/libraries/PathKey.sol index a286076d3..b3fa1e7f8 100644 --- a/src/libraries/PathKey.sol +++ b/src/libraries/PathKey.sol @@ -1,5 +1,5 @@ //SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.20; +pragma solidity ^0.8.0; import {Currency} from "@uniswap/v4-core/src/types/Currency.sol"; import {IHooks} from "@uniswap/v4-core/src/interfaces/IHooks.sol"; @@ -13,7 +13,14 @@ struct PathKey { bytes hookData; } -library PathKeyLib { +/// @title PathKey Library +/// @notice Functions for working with PathKeys +library PathKeyLibrary { + /// @notice Get the pool and swap direction for a given PathKey + /// @param params the given PathKey + /// @param currencyIn the input currency + /// @return poolKey the pool key of the swap + /// @return zeroForOne the direction of the swap, true if currency0 is being swapped for currency1 function getPoolAndSwapDirection(PathKey calldata params, Currency currencyIn) internal pure diff --git a/src/libraries/PoolTicksCounter.sol b/src/libraries/PoolTicksCounter.sol index 49b3ff360..c101a22fa 100644 --- a/src/libraries/PoolTicksCounter.sol +++ b/src/libraries/PoolTicksCounter.sol @@ -1,11 +1,13 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity >=0.8.20; +pragma solidity ^0.8.0; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; import {PoolIdLibrary} from "@uniswap/v4-core/src/types/PoolId.sol"; import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; +/// @title Pool Ticks Counter +/// @notice Functions for counting the number of initialized ticks between two ticks library PoolTicksCounter { using PoolIdLibrary for PoolKey; using StateLibrary for IPoolManager; @@ -19,10 +21,16 @@ library PoolTicksCounter { bool tickAfterInitialized; } + /// @notice Count the number of initialized ticks between two ticks /// @dev This function counts the number of initialized ticks that would incur a gas cost between tickBefore and tickAfter. /// When tickBefore and/or tickAfter themselves are initialized, the logic over whether we should count them depends on the /// direction of the swap. If we are swapping upwards (tickAfter > tickBefore) we don't want to count tickBefore but we do /// want to count tickAfter. The opposite is true if we are swapping downwards. + /// @param self the IPoolManager + /// @param key the PoolKey of the pool + /// @param tickBefore the tick before the swap + /// @param tickAfter the tick after the swap + /// @return initializedTicksLoaded the number of initialized ticks loaded function countInitializedTicksLoaded(IPoolManager self, PoolKey memory key, int24 tickBefore, int24 tickAfter) internal view @@ -94,6 +102,8 @@ library PoolTicksCounter { return initializedTicksLoaded; } + /// @notice Count the number of set bits in a uint256 + /// @param x the uint256 to count the bits of function countOneBits(uint256 x) private pure returns (uint16) { uint16 bits = 0; while (x != 0) { diff --git a/src/libraries/PositionConfig.sol b/src/libraries/PositionConfig.sol index 8f3d6ead9..007e7bb9d 100644 --- a/src/libraries/PositionConfig.sol +++ b/src/libraries/PositionConfig.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; diff --git a/src/libraries/PositionConfigId.sol b/src/libraries/PositionConfigId.sol index 40127102a..4e31c760c 100644 --- a/src/libraries/PositionConfigId.sol +++ b/src/libraries/PositionConfigId.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.24; +pragma solidity ^0.8.0; /// @notice A configId is set per tokenId /// The lower 255 bits are used to store the truncated hash of the corresponding PositionConfig 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/erc721Permit/ERC721Permit.permit.t.sol b/test/erc721Permit/ERC721Permit.permit.t.sol index 654c20c93..e567d08f6 100644 --- a/test/erc721Permit/ERC721Permit.permit.t.sol +++ b/test/erc721Permit/ERC721Permit.permit.t.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.24; import "forge-std/Test.sol"; import {SignatureVerification} from "permit2/src/libraries/SignatureVerification.sol"; -import {ERC721PermitHashLibrary} from "../../src/libraries/ERC721PermitHash.sol"; +import {ERC721PermitHash} from "../../src/libraries/ERC721PermitHash.sol"; import {MockERC721Permit} from "../mocks/MockERC721Permit.sol"; import {IERC721Permit_v4} from "../../src/interfaces/IERC721Permit_v4.sol"; import {IERC721} from "forge-std/interfaces/IERC721.sol"; @@ -61,15 +61,15 @@ contract ERC721PermitTest is Test { // --- Test the signature-based approvals (permit) --- function test_permitTypeHash() public pure { assertEq( - ERC721PermitHashLibrary.PERMIT_TYPEHASH, + ERC721PermitHash.PERMIT_TYPEHASH, keccak256("Permit(address spender,uint256 tokenId,uint256 nonce,uint256 deadline)") ); } function test_fuzz_permitHash(address spender, uint256 tokenId, uint256 nonce, uint256 deadline) public pure { bytes32 expectedHash = - keccak256(abi.encode(ERC721PermitHashLibrary.PERMIT_TYPEHASH, spender, tokenId, nonce, deadline)); - assertEq(expectedHash, ERC721PermitHashLibrary.hashPermit(spender, tokenId, nonce, deadline)); + keccak256(abi.encode(ERC721PermitHash.PERMIT_TYPEHASH, spender, tokenId, nonce, deadline)); + assertEq(expectedHash, ERC721PermitHash.hashPermit(spender, tokenId, nonce, deadline)); } function test_domainSeparator() public view { @@ -284,7 +284,7 @@ contract ERC721PermitTest is Test { abi.encodePacked( "\x19\x01", erc721Permit.DOMAIN_SEPARATOR(), - keccak256(abi.encode(ERC721PermitHashLibrary.PERMIT_TYPEHASH, spender, tokenId, nonce, deadline)) + keccak256(abi.encode(ERC721PermitHash.PERMIT_TYPEHASH, spender, tokenId, nonce, deadline)) ) ); } diff --git a/test/erc721Permit/ERC721Permit.permitForAll.t.sol b/test/erc721Permit/ERC721Permit.permitForAll.t.sol index 26c97a463..63e99ad31 100644 --- a/test/erc721Permit/ERC721Permit.permitForAll.t.sol +++ b/test/erc721Permit/ERC721Permit.permitForAll.t.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.24; import "forge-std/Test.sol"; import {SignatureVerification} from "permit2/src/libraries/SignatureVerification.sol"; -import {ERC721PermitHashLibrary} from "../../src/libraries/ERC721PermitHash.sol"; +import {ERC721PermitHash} from "../../src/libraries/ERC721PermitHash.sol"; import {MockERC721Permit} from "../mocks/MockERC721Permit.sol"; import {IERC721Permit_v4} from "../../src/interfaces/IERC721Permit_v4.sol"; import {IERC721} from "forge-std/interfaces/IERC721.sol"; @@ -51,15 +51,15 @@ contract ERC721PermitForAllTest is Test { // --- Test the signature-based approvals (permitForAll) --- function test_permitForAllTypeHash() public pure { assertEq( - ERC721PermitHashLibrary.PERMIT_FOR_ALL_TYPEHASH, + ERC721PermitHash.PERMIT_FOR_ALL_TYPEHASH, keccak256("PermitForAll(address operator,bool approved,uint256 nonce,uint256 deadline)") ); } function test_fuzz_permitForAllHash(address operator, bool approved, uint256 nonce, uint256 deadline) public pure { bytes32 expectedHash = - keccak256(abi.encode(ERC721PermitHashLibrary.PERMIT_FOR_ALL_TYPEHASH, operator, approved, nonce, deadline)); - assertEq(expectedHash, ERC721PermitHashLibrary.hashPermitForAll(operator, approved, nonce, deadline)); + keccak256(abi.encode(ERC721PermitHash.PERMIT_FOR_ALL_TYPEHASH, operator, approved, nonce, deadline)); + assertEq(expectedHash, ERC721PermitHash.hashPermitForAll(operator, approved, nonce, deadline)); } /// @dev operator uses alice's signature to approve itself @@ -338,9 +338,7 @@ contract ERC721PermitForAllTest is Test { abi.encodePacked( "\x19\x01", erc721Permit.DOMAIN_SEPARATOR(), - keccak256( - abi.encode(ERC721PermitHashLibrary.PERMIT_FOR_ALL_TYPEHASH, operator, approved, nonce, deadline) - ) + keccak256(abi.encode(ERC721PermitHash.PERMIT_FOR_ALL_TYPEHASH, operator, approved, nonce, deadline)) ) ); } @@ -354,7 +352,7 @@ contract ERC721PermitForAllTest is Test { abi.encodePacked( "\x19\x01", erc721Permit.DOMAIN_SEPARATOR(), - keccak256(abi.encode(ERC721PermitHashLibrary.PERMIT_TYPEHASH, spender, tokenId, nonce, deadline)) + keccak256(abi.encode(ERC721PermitHash.PERMIT_TYPEHASH, spender, tokenId, nonce, deadline)) ) ); } diff --git a/test/mocks/MockBadSubscribers.sol b/test/mocks/MockBadSubscribers.sol index 30bafba79..b5fd64f40 100644 --- a/test/mocks/MockBadSubscribers.sol +++ b/test/mocks/MockBadSubscribers.sol @@ -34,7 +34,7 @@ contract MockReturnDataSubscriber is ISubscriber { notifySubscribeCount++; } - function notifyUnsubscribe(uint256, PositionConfig memory, bytes memory) external onlyByPosm { + function notifyUnsubscribe(uint256, PositionConfig memory) external onlyByPosm { notifyUnsubscribeCount++; uint256 _memPtr = memPtr; assembly { @@ -83,7 +83,7 @@ contract MockRevertSubscriber is ISubscriber { } } - function notifyUnsubscribe(uint256, PositionConfig memory, bytes memory) external view onlyByPosm { + function notifyUnsubscribe(uint256, PositionConfig memory) external view onlyByPosm { revert TestRevert("notifyUnsubscribe"); } diff --git a/test/mocks/MockSubscriber.sol b/test/mocks/MockSubscriber.sol index 032bea868..83ea2e39c 100644 --- a/test/mocks/MockSubscriber.sol +++ b/test/mocks/MockSubscriber.sol @@ -18,7 +18,6 @@ contract MockSubscriber is ISubscriber { BalanceDelta public feesAccrued; bytes public subscribeData; - bytes public unsubscribeData; error NotAuthorizedNotifer(address sender); @@ -38,9 +37,8 @@ contract MockSubscriber is ISubscriber { subscribeData = data; } - function notifyUnsubscribe(uint256, PositionConfig memory, bytes memory data) external onlyByPosm { + function notifyUnsubscribe(uint256, PositionConfig memory) external onlyByPosm { notifyUnsubscribeCount++; - unsubscribeData = data; } function notifyModifyLiquidity(uint256, PositionConfig memory, int256 _liquidityChange, BalanceDelta _feesAccrued) 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.gas.t.sol b/test/position-managers/PositionManager.gas.t.sol index 161321eac..2463b7102 100644 --- a/test/position-managers/PositionManager.gas.t.sol +++ b/test/position-managers/PositionManager.gas.t.sol @@ -24,6 +24,7 @@ import {IMulticall_v4} from "../../src/interfaces/IMulticall_v4.sol"; import {Planner, Plan} from "../shared/Planner.sol"; import {PosmTestSetup} from "../shared/PosmTestSetup.sol"; import {ActionConstants} from "../../src/libraries/ActionConstants.sol"; +import {MockSubscriber} from "../mocks/MockSubscriber.sol"; contract PosMGasTest is Test, PosmTestSetup, GasSnapshot { using FixedPointMathLib for uint256; @@ -43,6 +44,8 @@ contract PosMGasTest is Test, PosmTestSetup, GasSnapshot { PositionConfig config; PositionConfig configNative; + MockSubscriber sub; + function setUp() public { (alice, alicePK) = makeAddrAndKey("ALICE"); (bob, bobPK) = makeAddrAndKey("BOB"); @@ -68,6 +71,8 @@ contract PosMGasTest is Test, PosmTestSetup, GasSnapshot { // define a reusable range config = PositionConfig({poolKey: key, tickLower: -300, tickUpper: 300}); configNative = PositionConfig({poolKey: nativeKey, tickLower: -300, tickUpper: 300}); + + sub = new MockSubscriber(lpm); } function test_gas_mint_withClose() public { @@ -850,4 +855,15 @@ contract PosMGasTest is Test, PosmTestSetup, GasSnapshot { lpm.modifyLiquidities(calls, _deadline); snapLastCall("PositionManager_decrease_take_take"); } + + function test_gas_subscribe_unsubscribe() public { + uint256 tokenId = lpm.nextTokenId(); + mint(config, 1e18, ActionConstants.MSG_SENDER, ZERO_BYTES); + + lpm.subscribe(tokenId, config, address(sub), ZERO_BYTES); + snapLastCall("PositionManager_subscribe"); + + lpm.unsubscribe(tokenId, config); + snapLastCall("PositionManager_unsubscribe"); + } } 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 48ba5bced..d06ed52d5 100644 --- a/test/position-managers/PositionManager.notifier.t.sol +++ b/test/position-managers/PositionManager.notifier.t.sol @@ -97,6 +97,41 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { assertEq(sub.notifySubscribeCount(), 1); } + /// @notice Revert when subscribing to an address without code + function test_subscribe_revert_empty(address _subscriber) public { + vm.assume(_subscriber.code.length == 0); + + 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(); + + vm.expectRevert(INotifier.NoCodeSubscriber.selector); + lpm.subscribe(tokenId, config, _subscriber, ZERO_BYTES); + } + + 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); @@ -126,6 +161,28 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { assertEq(sub.notifyModifyLiquidityCount(), 10); } + function test_notifyModifyLiquidity_selfDestruct_revert() 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(); + + lpm.subscribe(tokenId, config, address(sub), ZERO_BYTES); + + assertEq(lpm.hasSubscriber(tokenId), true); + assertEq(address(lpm.subscriber(tokenId)), address(sub)); + + // simulate selfdestruct by etching the bytecode to 0 + vm.etch(address(sub), ZERO_BYTES); + + uint256 liquidityToAdd = 10e18; + vm.expectRevert(INotifier.NoCodeSubscriber.selector); + increaseLiquidity(tokenId, config, liquidityToAdd, ZERO_BYTES); + } + function test_notifyModifyLiquidity_args() public { uint256 tokenId = lpm.nextTokenId(); mint(config, 100e18, alice, ZERO_BYTES); @@ -173,6 +230,26 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { assertEq(sub.notifyTransferCount(), 1); } + function test_notifyTransfer_withTransferFrom_selfDestruct_revert() 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(); + + lpm.subscribe(tokenId, config, address(sub), ZERO_BYTES); + assertEq(lpm.hasSubscriber(tokenId), true); + assertEq(address(lpm.subscriber(tokenId)), address(sub)); + + // simulate selfdestruct by etching the bytecode to 0 + vm.etch(address(sub), ZERO_BYTES); + + vm.expectRevert(INotifier.NoCodeSubscriber.selector); + lpm.transferFrom(alice, bob, tokenId); + } + function test_notifyTransfer_withSafeTransferFrom_succeeds() public { uint256 tokenId = lpm.nextTokenId(); mint(config, 100e18, alice, ZERO_BYTES); @@ -192,6 +269,26 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { assertEq(sub.notifyTransferCount(), 1); } + function test_notifyTransfer_withSafeTransferFrom_selfDestruct_revert() 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(); + + lpm.subscribe(tokenId, config, address(sub), ZERO_BYTES); + assertEq(lpm.hasSubscriber(tokenId), true); + assertEq(address(lpm.subscriber(tokenId)), address(sub)); + + // simulate selfdestruct by etching the bytecode to 0 + vm.etch(address(sub), ZERO_BYTES); + + vm.expectRevert(INotifier.NoCodeSubscriber.selector); + lpm.safeTransferFrom(alice, bob, tokenId); + } + function test_notifyTransfer_withSafeTransferFromData_succeeds() public { uint256 tokenId = lpm.nextTokenId(); mint(config, 100e18, alice, ZERO_BYTES); @@ -222,7 +319,7 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { lpm.subscribe(tokenId, config, address(sub), ZERO_BYTES); - lpm.unsubscribe(tokenId, config, ZERO_BYTES); + lpm.unsubscribe(tokenId, config); assertEq(sub.notifyUnsubscribeCount(), 1); assertEq(lpm.hasSubscriber(tokenId), false); @@ -241,7 +338,7 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { lpm.subscribe(tokenId, config, address(badSubscriber), ZERO_BYTES); MockReturnDataSubscriber(badSubscriber).setReturnDataSize(0x600000); - lpm.unsubscribe(tokenId, config, ZERO_BYTES); + lpm.unsubscribe(tokenId, config); // the subscriber contract call failed bc it used too much gas assertEq(MockReturnDataSubscriber(badSubscriber).notifyUnsubscribeCount(), 0); @@ -249,6 +346,26 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { assertEq(address(lpm.subscriber(tokenId)), address(0)); } + function test_unsubscribe_selfDestructed() 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(); + + lpm.subscribe(tokenId, config, address(sub), ZERO_BYTES); + + // simulate selfdestruct by etching the bytecode to 0 + vm.etch(address(sub), ZERO_BYTES); + + lpm.unsubscribe(tokenId, config); + + assertEq(lpm.hasSubscriber(tokenId), false); + assertEq(address(lpm.subscriber(tokenId)), address(0)); + } + function test_multicall_mint_subscribe() public { uint256 tokenId = lpm.nextTokenId(); @@ -320,30 +437,28 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { lpm.approve(address(this), tokenId); vm.stopPrank(); - vm.expectRevert(); - lpm.unsubscribe(tokenId, config, ZERO_BYTES); + vm.expectRevert(INotifier.NotSubscribed.selector); + lpm.unsubscribe(tokenId, config); } - function test_subscribe_withData() public { + function test_unsubscribe_twice_reverts() public { uint256 tokenId = lpm.nextTokenId(); mint(config, 100e18, alice, ZERO_BYTES); - bytes memory subData = abi.encode(address(this)); - // approve this contract to operate on alices liq vm.startPrank(alice); lpm.approve(address(this), tokenId); vm.stopPrank(); - lpm.subscribe(tokenId, config, address(sub), subData); + lpm.subscribe(tokenId, config, address(sub), ZERO_BYTES); - assertEq(lpm.hasSubscriber(tokenId), true); - assertEq(address(lpm.subscriber(tokenId)), address(sub)); - assertEq(sub.notifySubscribeCount(), 1); - assertEq(abi.decode(sub.subscribeData(), (address)), address(this)); + lpm.unsubscribe(tokenId, config); + + vm.expectRevert(INotifier.NotSubscribed.selector); + lpm.unsubscribe(tokenId, config); } - function test_unsubscribe_withData() public { + function test_subscribe_withData() public { uint256 tokenId = lpm.nextTokenId(); mint(config, 100e18, alice, ZERO_BYTES); @@ -354,14 +469,12 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { lpm.approve(address(this), tokenId); vm.stopPrank(); - lpm.subscribe(tokenId, config, address(sub), ZERO_BYTES); - - lpm.unsubscribe(tokenId, config, subData); + lpm.subscribe(tokenId, config, address(sub), subData); - assertEq(sub.notifyUnsubscribeCount(), 1); - assertEq(lpm.hasSubscriber(tokenId), false); - assertEq(address(lpm.subscriber(tokenId)), address(0)); - assertEq(abi.decode(sub.unsubscribeData(), (address)), address(this)); + assertEq(lpm.hasSubscriber(tokenId), true); + assertEq(address(lpm.subscriber(tokenId)), address(sub)); + assertEq(sub.notifySubscribeCount(), 1); + assertEq(abi.decode(sub.subscribeData(), (address)), address(this)); } function test_subscribe_wraps_revert() public { @@ -477,4 +590,56 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { ); lpm.safeTransferFrom(alice, bob, tokenId, ""); } + + /// @notice burning a position will automatically notify unsubscribe + function test_burn_unsubscribe() public { + uint256 tokenId = lpm.nextTokenId(); + mint(config, 100e18, alice, ZERO_BYTES); + + bytes memory subData = abi.encode(address(this)); + + // approve this contract to operate on alices liq + vm.startPrank(alice); + lpm.approve(address(this), tokenId); + vm.stopPrank(); + + lpm.subscribe(tokenId, config, address(sub), subData); + + assertEq(lpm.hasSubscriber(tokenId), true); + assertEq(sub.notifyUnsubscribeCount(), 0); + + // burn the position, causing an unsubscribe + burn(tokenId, config, ZERO_BYTES); + + // position is now unsubscribed + assertEq(lpm.hasSubscriber(tokenId), false); + assertEq(sub.notifyUnsubscribeCount(), 1); + } + + /// @notice Test that users cannot forcibly avoid unsubscribe logic via gas limits + function test_fuzz_unsubscribe_with_gas_limit(uint64 gasLimit) public { + // enforce a minimum amount of gas to avoid OutOfGas reverts + gasLimit = uint64(bound(gasLimit, 125_000, block.gaslimit)); + + 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(); + + lpm.subscribe(tokenId, config, address(sub), ZERO_BYTES); + uint256 beforeUnsubCount = sub.notifyUnsubscribeCount(); + + if (gasLimit < lpm.unsubscribeGasLimit()) { + // gas too low to call a valid unsubscribe + vm.expectRevert(INotifier.GasLimitTooLow.selector); + lpm.unsubscribe{gas: gasLimit}(tokenId, config); + } else { + // increasing gas limit succeeds and unsubscribe was called + lpm.unsubscribe{gas: gasLimit}(tokenId, config); + assertEq(sub.notifyUnsubscribeCount(), beforeUnsubCount + 1); + } + } } 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); } diff --git a/test/shared/PosmTestSetup.sol b/test/shared/PosmTestSetup.sol index 345726c4d..81c4f9bf5 100644 --- a/test/shared/PosmTestSetup.sol +++ b/test/shared/PosmTestSetup.sol @@ -15,7 +15,7 @@ import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol" import {DeployPermit2} from "permit2/test/utils/DeployPermit2.sol"; import {HookSavesDelta} from "./HookSavesDelta.sol"; import {HookModifyLiquidities} from "./HookModifyLiquidities.sol"; -import {ERC721PermitHashLibrary} from "../../src/libraries/ERC721PermitHash.sol"; +import {ERC721PermitHash} from "../../src/libraries/ERC721PermitHash.sol"; /// @notice A shared test contract that wraps the v4-core deployers contract and exposes basic liquidity operations on posm. contract PosmTestSetup is Test, Deployers, DeployPermit2, LiquidityOperations { @@ -57,7 +57,7 @@ contract PosmTestSetup is Test, Deployers, DeployPermit2, LiquidityOperations { function deployPosm(IPoolManager poolManager) internal { // We use deployPermit2() to prevent having to use via-ir in this repository. permit2 = IAllowanceTransfer(deployPermit2()); - lpm = new PositionManager(poolManager, permit2); + lpm = new PositionManager(poolManager, permit2, 100_000); } function seedBalance(address to) internal { @@ -104,7 +104,7 @@ contract PosmTestSetup is Test, Deployers, DeployPermit2, LiquidityOperations { abi.encodePacked( "\x19\x01", lpm.DOMAIN_SEPARATOR(), - keccak256(abi.encode(ERC721PermitHashLibrary.PERMIT_TYPEHASH, spender, tokenId, nonce, deadline)) + keccak256(abi.encode(ERC721PermitHash.PERMIT_TYPEHASH, spender, tokenId, nonce, deadline)) ) ); }