From 6f5f3f2d20c2ff1076b989fdc07ec8317d28dca1 Mon Sep 17 00:00:00 2001 From: Alice <34962750+hensha256@users.noreply.github.com> Date: Wed, 28 Feb 2024 22:22:43 -0300 Subject: [PATCH] Remove nested locking, access lock, and lockCaller (#442) * Remove access lock * remove current hook * Remove nested locking * Locker library tests * Rename empty lock test * Test skeleton * Test for a nested swap * Tests for nested function calls * snapshots * Separate libs, rename error * Remove lock caller * merge errors * Update test/NonZeroDeltaCount.t.sol Co-authored-by: Sara Reynolds <30504811+snreynolds@users.noreply.github.com> * Update src/libraries/NonZeroDeltaCount.sol Co-authored-by: Sara Reynolds <30504811+snreynolds@users.noreply.github.com> * test renaming --------- Co-authored-by: Sara Reynolds <30504811+snreynolds@users.noreply.github.com> --- .../addLiquidity with empty hook.snap | 2 +- .../addLiquidity with native token.snap | 2 +- .forge-snapshots/addLiquidity.snap | 2 +- ...swap hook, already cached dynamic fee.snap | 2 +- .../cached dynamic fee, no hooks.snap | 2 +- .forge-snapshots/donate gas with 1 token.snap | 2 +- .../donate gas with 2 tokens.snap | 2 +- .../gas overhead of no-op lock.snap | 2 +- .forge-snapshots/initialize.snap | 2 +- .../mintWithEmptyHookEOAInitiated.snap | 2 +- .../modify position with noop.snap | 2 +- .../poolManager bytecode size.snap | 2 +- .../removeLiquidity with empty hook.snap | 2 +- .../removeLiquidity with native token.snap | 2 +- .forge-snapshots/removeLiquidity.snap | 2 +- .forge-snapshots/simple swap with native.snap | 2 +- .forge-snapshots/simple swap.snap | 2 +- .forge-snapshots/simpleSwapEOAInitiated.snap | 2 +- .../simpleSwapNativeEOAInitiated.snap | 2 +- ...p against liquidity with native token.snap | 2 +- .forge-snapshots/swap against liquidity.snap | 2 +- .../swap burn 6909 for input.snap | 2 +- .../swap burn native 6909 for input.snap | 2 +- .../swap mint native output as 6909.snap | 2 +- .../swap mint output as 6909.snap | 2 +- .forge-snapshots/swap with dynamic fee.snap | 2 +- .forge-snapshots/swap with hooks.snap | 2 +- .forge-snapshots/swap with noop.snap | 2 +- .../update dynamic fee in before swap.snap | 2 +- src/PoolManager.sol | 65 +- src/interfaces/IPoolManager.sol | 19 +- src/libraries/Hooks.sol | 12 +- src/libraries/Locker.sol | 44 + src/libraries/Lockers.sol | 153 --- src/libraries/NonZeroDeltaCount.sol | 39 + src/test/AccessLockHook.sol | 304 ------ src/test/EmptyTestHooks.sol | 3 +- src/test/NoOpTestHooks.sol | 3 +- src/test/PoolDonateTest.sol | 25 +- ...PoolLockTest.sol => PoolEmptyLockTest.sol} | 2 +- src/test/PoolInitializeTest.sol | 14 +- src/test/PoolModifyLiquidityTest.sol | 21 +- src/test/PoolNestedActionsTest.sol | 214 +++++ src/test/PoolSwapTest.sol | 63 +- src/test/PoolTakeTest.sol | 4 +- src/test/PoolTestBase.sol | 4 +- test/AccessLock.t.sol | 901 ------------------ test/CurrentHookAddress.t.sol | 46 - test/Hooks.t.sol | 117 +-- test/Locker.t.sol | 46 + test/LockersLibrary.t.sol | 148 --- test/NestedActions.t.sol | 38 + test/NonZeroDeltaCount.t.sol | 41 + test/PoolManager.t.sol | 46 +- test/PoolManagerInitialize.t.sol | 14 +- test/PoolManagerReentrancyTest.t.sol | 171 ---- test/utils/Constants.sol | 6 + test/utils/Deployers.sol | 18 +- test/utils/SwapHelper.t.sol | 1 - 59 files changed, 617 insertions(+), 2023 deletions(-) create mode 100644 src/libraries/Locker.sol delete mode 100644 src/libraries/Lockers.sol create mode 100644 src/libraries/NonZeroDeltaCount.sol delete mode 100644 src/test/AccessLockHook.sol rename src/test/{PoolLockTest.sol => PoolEmptyLockTest.sol} (93%) create mode 100644 src/test/PoolNestedActionsTest.sol delete mode 100644 test/AccessLock.t.sol delete mode 100644 test/CurrentHookAddress.t.sol create mode 100644 test/Locker.t.sol delete mode 100644 test/LockersLibrary.t.sol create mode 100644 test/NestedActions.t.sol create mode 100644 test/NonZeroDeltaCount.t.sol delete mode 100644 test/PoolManagerReentrancyTest.t.sol diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap index 57d8bdab0..cfc6e3e4e 100644 --- a/.forge-snapshots/addLiquidity with empty hook.snap +++ b/.forge-snapshots/addLiquidity with empty hook.snap @@ -1 +1 @@ -323570 \ No newline at end of file +313034 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap index ba33dfb20..de7c2ff33 100644 --- a/.forge-snapshots/addLiquidity with native token.snap +++ b/.forge-snapshots/addLiquidity with native token.snap @@ -1 +1 @@ -200465 \ No newline at end of file +193189 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity.snap b/.forge-snapshots/addLiquidity.snap index 1323d2947..1750a6692 100644 --- a/.forge-snapshots/addLiquidity.snap +++ b/.forge-snapshots/addLiquidity.snap @@ -1 +1 @@ -200443 \ No newline at end of file +193167 \ No newline at end of file diff --git a/.forge-snapshots/before swap hook, already cached dynamic fee.snap b/.forge-snapshots/before swap hook, already cached dynamic fee.snap index e8e52470b..3cbc98f73 100644 --- a/.forge-snapshots/before swap hook, already cached dynamic fee.snap +++ b/.forge-snapshots/before swap hook, already cached dynamic fee.snap @@ -1 +1 @@ -193921 \ No newline at end of file +185454 \ No newline at end of file diff --git a/.forge-snapshots/cached dynamic fee, no hooks.snap b/.forge-snapshots/cached dynamic fee, no hooks.snap index 1eca261d2..298079c1f 100644 --- a/.forge-snapshots/cached dynamic fee, no hooks.snap +++ b/.forge-snapshots/cached dynamic fee, no hooks.snap @@ -1 +1 @@ -146291 \ No newline at end of file +139454 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 1 token.snap b/.forge-snapshots/donate gas with 1 token.snap index 9b1b58a44..2923d0ce4 100644 --- a/.forge-snapshots/donate gas with 1 token.snap +++ b/.forge-snapshots/donate gas with 1 token.snap @@ -1 +1 @@ -137497 \ No newline at end of file +132530 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 2 tokens.snap b/.forge-snapshots/donate gas with 2 tokens.snap index 826dd7b9f..8b6772d67 100644 --- a/.forge-snapshots/donate gas with 2 tokens.snap +++ b/.forge-snapshots/donate gas with 2 tokens.snap @@ -1 +1 @@ -184854 \ No newline at end of file +177634 \ No newline at end of file diff --git a/.forge-snapshots/gas overhead of no-op lock.snap b/.forge-snapshots/gas overhead of no-op lock.snap index c01572647..55ffc0c6e 100644 --- a/.forge-snapshots/gas overhead of no-op lock.snap +++ b/.forge-snapshots/gas overhead of no-op lock.snap @@ -1 +1 @@ -15181 \ No newline at end of file +14810 \ No newline at end of file diff --git a/.forge-snapshots/initialize.snap b/.forge-snapshots/initialize.snap index 1810dd3a6..d9dfed896 100644 --- a/.forge-snapshots/initialize.snap +++ b/.forge-snapshots/initialize.snap @@ -1 +1 @@ -74139 \ No newline at end of file +72764 \ No newline at end of file diff --git a/.forge-snapshots/mintWithEmptyHookEOAInitiated.snap b/.forge-snapshots/mintWithEmptyHookEOAInitiated.snap index 07bdef5b1..2257feb13 100644 --- a/.forge-snapshots/mintWithEmptyHookEOAInitiated.snap +++ b/.forge-snapshots/mintWithEmptyHookEOAInitiated.snap @@ -1 +1 @@ -255434 \ No newline at end of file +247151 \ No newline at end of file diff --git a/.forge-snapshots/modify position with noop.snap b/.forge-snapshots/modify position with noop.snap index 84e7c9836..84e389dfe 100644 --- a/.forge-snapshots/modify position with noop.snap +++ b/.forge-snapshots/modify position with noop.snap @@ -1 +1 @@ -58501 \ No newline at end of file +54101 \ No newline at end of file diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap index ba031d117..09ce280b6 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -24361 \ No newline at end of file +23847 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap index deef11241..d2c53fce5 100644 --- a/.forge-snapshots/removeLiquidity with empty hook.snap +++ b/.forge-snapshots/removeLiquidity with empty hook.snap @@ -1 +1 @@ -107256 \ No newline at end of file +99936 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap index 9cd68f961..baf590c91 100644 --- a/.forge-snapshots/removeLiquidity with native token.snap +++ b/.forge-snapshots/removeLiquidity with native token.snap @@ -1 +1 @@ -208629 \ No newline at end of file +201309 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity.snap b/.forge-snapshots/removeLiquidity.snap index e204e785c..04918ec24 100644 --- a/.forge-snapshots/removeLiquidity.snap +++ b/.forge-snapshots/removeLiquidity.snap @@ -1 +1 @@ -204906 \ No newline at end of file +197586 \ No newline at end of file diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap index 3d4a08ff0..6f27b91e1 100644 --- a/.forge-snapshots/simple swap with native.snap +++ b/.forge-snapshots/simple swap with native.snap @@ -1 +1 @@ -195291 \ No newline at end of file +188027 \ No newline at end of file diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap index 32e652e18..7ce4b83c1 100644 --- a/.forge-snapshots/simple swap.snap +++ b/.forge-snapshots/simple swap.snap @@ -1 +1 @@ -203828 \ No newline at end of file +196564 \ No newline at end of file diff --git a/.forge-snapshots/simpleSwapEOAInitiated.snap b/.forge-snapshots/simpleSwapEOAInitiated.snap index 165d11298..38c1a1eb5 100644 --- a/.forge-snapshots/simpleSwapEOAInitiated.snap +++ b/.forge-snapshots/simpleSwapEOAInitiated.snap @@ -1 +1 @@ -173759 \ No newline at end of file +166450 \ No newline at end of file diff --git a/.forge-snapshots/simpleSwapNativeEOAInitiated.snap b/.forge-snapshots/simpleSwapNativeEOAInitiated.snap index 357ca25d6..aaea2d8e3 100644 --- a/.forge-snapshots/simpleSwapNativeEOAInitiated.snap +++ b/.forge-snapshots/simpleSwapNativeEOAInitiated.snap @@ -1 +1 @@ -172441 \ No newline at end of file +165132 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap index de042abd8..0946afcde 100644 --- a/.forge-snapshots/swap against liquidity with native token.snap +++ b/.forge-snapshots/swap against liquidity with native token.snap @@ -1 +1 @@ -125954 \ No newline at end of file +118690 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap index 300b2a479..210a6bf3b 100644 --- a/.forge-snapshots/swap against liquidity.snap +++ b/.forge-snapshots/swap against liquidity.snap @@ -1 +1 @@ -113418 \ No newline at end of file +106154 \ No newline at end of file diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap index 179b18b8a..ab4b64fdb 100644 --- a/.forge-snapshots/swap burn 6909 for input.snap +++ b/.forge-snapshots/swap burn 6909 for input.snap @@ -1 +1 @@ -133630 \ No newline at end of file +126344 \ No newline at end of file diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap index 5a7e14b1f..2baddebf2 100644 --- a/.forge-snapshots/swap burn native 6909 for input.snap +++ b/.forge-snapshots/swap burn native 6909 for input.snap @@ -1 +1 @@ -129583 \ No newline at end of file +122297 \ No newline at end of file diff --git a/.forge-snapshots/swap mint native output as 6909.snap b/.forge-snapshots/swap mint native output as 6909.snap index 9db6bbf75..f858065c5 100644 --- a/.forge-snapshots/swap mint native output as 6909.snap +++ b/.forge-snapshots/swap mint native output as 6909.snap @@ -1 +1 @@ -197863 \ No newline at end of file +190554 \ No newline at end of file diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap index 170828963..3ad1208f2 100644 --- a/.forge-snapshots/swap mint output as 6909.snap +++ b/.forge-snapshots/swap mint output as 6909.snap @@ -1 +1 @@ -214668 \ No newline at end of file +207359 \ No newline at end of file diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap index 1a7fded29..90130ac49 100644 --- a/.forge-snapshots/swap with dynamic fee.snap +++ b/.forge-snapshots/swap with dynamic fee.snap @@ -1 +1 @@ -193022 \ No newline at end of file +184555 \ No newline at end of file diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap index a000fca5c..3803f5a6e 100644 --- a/.forge-snapshots/swap with hooks.snap +++ b/.forge-snapshots/swap with hooks.snap @@ -1 +1 @@ -113396 \ No newline at end of file +106132 \ No newline at end of file diff --git a/.forge-snapshots/swap with noop.snap b/.forge-snapshots/swap with noop.snap index 1ac782382..cd37cba40 100644 --- a/.forge-snapshots/swap with noop.snap +++ b/.forge-snapshots/swap with noop.snap @@ -1 +1 @@ -51199 \ No newline at end of file +46833 \ No newline at end of file diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap index 464e42ce0..849cdfec2 100644 --- a/.forge-snapshots/update dynamic fee in before swap.snap +++ b/.forge-snapshots/update dynamic fee in before swap.snap @@ -1 +1 @@ -199770 \ No newline at end of file +191259 \ No newline at end of file diff --git a/src/PoolManager.sol b/src/PoolManager.sol index d18015b50..21b2fab1e 100644 --- a/src/PoolManager.sol +++ b/src/PoolManager.sol @@ -19,7 +19,8 @@ import {Fees} from "./Fees.sol"; import {ERC6909Claims} from "./ERC6909Claims.sol"; import {PoolId, PoolIdLibrary} from "./types/PoolId.sol"; import {BalanceDelta, BalanceDeltaLibrary} from "./types/BalanceDelta.sol"; -import {Lockers} from "./libraries/Lockers.sol"; +import {Locker} from "./libraries/Locker.sol"; +import {NonZeroDeltaCount} from "./libraries/NonZeroDeltaCount.sol"; import {PoolGetters} from "./libraries/PoolGetters.sol"; /// @notice Holds the state for all pools @@ -88,27 +89,21 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims { } /// @inheritdoc IPoolManager - function getLock(uint256 i) external view override returns (address locker, address lockCaller) { - return (Lockers.getLocker(i), Lockers.getLockCaller(i)); + function getLocker() external view override returns (address locker) { + return Locker.getLocker(); } /// @notice This will revert if a function is called by any address other than the current locker OR the most recently called, pre-permissioned hook. - modifier onlyByLocker() { - _checkLocker(msg.sender, Lockers.getCurrentLocker(), Lockers.getCurrentHook()); + modifier isLocked() { + if (!Locker.isLocked()) revert ManagerNotLocked(); _; } - function _checkLocker(address caller, address locker, IHooks hook) internal pure { - if (caller == locker) return; - if (caller == address(hook) && hook.hasPermission(Hooks.ACCESS_LOCK_FLAG)) return; - revert LockedBy(locker, address(hook)); - } - /// @inheritdoc IPoolManager function initialize(PoolKey memory key, uint160 sqrtPriceX96, bytes calldata hookData) external override - onlyByLocker + isLocked returns (int24 tick) { if (key.fee.isStaticFeeTooLarge()) revert FeeTooLarge(); @@ -135,35 +130,33 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims { /// @inheritdoc IPoolManager function lock(address lockTarget, bytes calldata data) external payable override returns (bytes memory result) { - Lockers.push(lockTarget, msg.sender); + // Get the lock caller because thats an EOA and is not user-controlable + if (Locker.isLocked()) revert AlreadyLocked(); + + Locker.setLocker(lockTarget); // the caller does everything in this callback, including paying what they owe via calls to settle result = ILockCallback(lockTarget).lockAcquired(msg.sender, data); - if (Lockers.length() == 1) { - if (Lockers.nonzeroDeltaCount() != 0) revert CurrencyNotSettled(); - Lockers.clear(); - } else { - Lockers.pop(); - } + if (NonZeroDeltaCount.read() != 0) revert CurrencyNotSettled(); + Locker.clearLocker(); } function _accountDelta(Currency currency, int128 delta) internal { if (delta == 0) return; - address locker = Lockers.getCurrentLocker(); - int256 current = currencyDelta[locker][currency]; + int256 current = currencyDelta[msg.sender][currency]; int256 next = current + delta; unchecked { if (next == 0) { - Lockers.decrementNonzeroDeltaCount(); + NonZeroDeltaCount.decrement(); } else if (current == 0) { - Lockers.incrementNonzeroDeltaCount(); + NonZeroDeltaCount.increment(); } } - currencyDelta[locker][currency] = next; + currencyDelta[msg.sender][currency] = next; } /// @dev Accumulates a balance change to a map of currency to balance changes @@ -181,7 +174,7 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims { PoolKey memory key, IPoolManager.ModifyLiquidityParams memory params, bytes calldata hookData - ) external override noDelegateCall onlyByLocker returns (BalanceDelta delta) { + ) external override noDelegateCall isLocked returns (BalanceDelta delta) { PoolId id = key.toId(); _checkPoolInitialized(id); @@ -211,7 +204,7 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims { external override noDelegateCall - onlyByLocker + isLocked returns (BalanceDelta delta) { PoolId id = key.toId(); @@ -254,7 +247,7 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims { external override noDelegateCall - onlyByLocker + isLocked returns (BalanceDelta delta) { PoolId id = key.toId(); @@ -272,14 +265,14 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims { } /// @inheritdoc IPoolManager - function take(Currency currency, address to, uint256 amount) external override noDelegateCall onlyByLocker { + function take(Currency currency, address to, uint256 amount) external override noDelegateCall isLocked { _accountDelta(currency, amount.toInt128()); reservesOf[currency] -= amount; currency.transfer(to, amount); } /// @inheritdoc IPoolManager - function settle(Currency currency) external payable override noDelegateCall onlyByLocker returns (uint256 paid) { + function settle(Currency currency) external payable override noDelegateCall isLocked returns (uint256 paid) { uint256 reservesBefore = reservesOf[currency]; reservesOf[currency] = currency.balanceOfSelf(); paid = reservesOf[currency] - reservesBefore; @@ -288,13 +281,13 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims { } /// @inheritdoc IPoolManager - function mint(address to, uint256 id, uint256 amount) external override noDelegateCall onlyByLocker { + function mint(address to, uint256 id, uint256 amount) external override noDelegateCall isLocked { _accountDelta(CurrencyLibrary.fromId(id), amount.toInt128()); _mint(to, id, amount); } /// @inheritdoc IPoolManager - function burn(address from, uint256 id, uint256 amount) external override noDelegateCall onlyByLocker { + function burn(address from, uint256 id, uint256 amount) external override noDelegateCall isLocked { _accountDelta(CurrencyLibrary.fromId(id), -(amount.toInt128())); _burnFrom(from, id, amount); } @@ -338,16 +331,8 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims { return value; } - function getLockLength() external view returns (uint256 _length) { - return Lockers.length(); - } - function getLockNonzeroDeltaCount() external view returns (uint256 _nonzeroDeltaCount) { - return Lockers.nonzeroDeltaCount(); - } - - function getCurrentHook() external view returns (IHooks) { - return Lockers.getCurrentHook(); + return NonZeroDeltaCount.read(); } function getPoolTickInfo(PoolId id, int24 tick) external view returns (Pool.TickInfo memory) { diff --git a/src/interfaces/IPoolManager.sol b/src/interfaces/IPoolManager.sol index 32e81ad5d..7e679b631 100644 --- a/src/interfaces/IPoolManager.sol +++ b/src/interfaces/IPoolManager.sol @@ -21,10 +21,11 @@ interface IPoolManager is IFees, IERC6909Claims { /// @notice Thrown when trying to interact with a non-initialized pool error PoolNotInitialized(); - /// @notice Thrown when a function is called by an address that is not the current locker - /// @param locker The current locker - /// @param currentHook The most recently called hook - error LockedBy(address locker, address currentHook); + /// @notice Thrown when lock is called, but a lock is already open + error AlreadyLocked(); + + /// @notice Thrown when a function is called outside of a lock + error ManagerNotLocked(); /// @notice The ERC1155 being deposited is not the Uniswap ERC1155 error NotPoolManagerToken(); @@ -119,14 +120,8 @@ interface IPoolManager is IFees, IERC6909Claims { /// @notice Returns the reserves for a given ERC20 currency function reservesOf(Currency currency) external view returns (uint256); - /// @notice Returns the locker in the ith position of the locker queue. - function getLock(uint256 i) external view returns (address locker, address lockCaller); - - /// @notice Returns the length of the lockers array, which is the number of locks open on the PoolManager. - function getLockLength() external view returns (uint256 _length); - - /// @notice Returns the most recently called hook. - function getCurrentHook() external view returns (IHooks _currentHook); + /// @notice Returns the locker of the pool + function getLocker() external view returns (address locker); /// @notice Returns the number of nonzero deltas open on the PoolManager that must be zerod by the close of the initial lock. function getLockNonzeroDeltaCount() external view returns (uint256 _nonzeroDeltaCount); diff --git a/src/libraries/Hooks.sol b/src/libraries/Hooks.sol index 3861424b0..969c38be9 100644 --- a/src/libraries/Hooks.sol +++ b/src/libraries/Hooks.sol @@ -6,7 +6,7 @@ import {IHooks} from "../interfaces/IHooks.sol"; import {FeeLibrary} from "./FeeLibrary.sol"; import {BalanceDelta} from "../types/BalanceDelta.sol"; import {IPoolManager} from "../interfaces/IPoolManager.sol"; -import {Lockers} from "./Lockers.sol"; +import {Locker} from "./Locker.sol"; /// @notice V4 decides whether to invoke specific hooks by inspecting the leading bits of the address that /// the hooks contract is deployed to. @@ -27,7 +27,6 @@ library Hooks { uint256 internal constant BEFORE_DONATE_FLAG = 1 << 151; uint256 internal constant AFTER_DONATE_FLAG = 1 << 150; uint256 internal constant NO_OP_FLAG = 1 << 149; - uint256 internal constant ACCESS_LOCK_FLAG = 1 << 148; bytes4 public constant NO_OP_SELECTOR = bytes4(keccak256(abi.encodePacked("NoOp"))); @@ -43,7 +42,6 @@ library Hooks { bool beforeDonate; bool afterDonate; bool noOp; - bool accessLock; } /// @notice Thrown if the address will not lead to the specified hook calls being called @@ -73,7 +71,6 @@ library Hooks { || permissions.beforeDonate != self.hasPermission(BEFORE_DONATE_FLAG) || permissions.afterDonate != self.hasPermission(AFTER_DONATE_FLAG) || permissions.noOp != self.hasPermission(NO_OP_FLAG) - || permissions.accessLock != self.hasPermission(ACCESS_LOCK_FLAG) ) { revert HookAddressNotValid(address(self)); } @@ -94,15 +91,13 @@ library Hooks { // If a hook contract is set, it must have at least 1 flag set, or have a dynamic fee return address(hook) == address(0) ? !fee.isDynamicFee() - : (uint160(address(hook)) >= ACCESS_LOCK_FLAG || fee.isDynamicFee()); + : (uint160(address(hook)) >= NO_OP_FLAG || fee.isDynamicFee()); } /// @notice performs a hook call using the given calldata on the given hook /// @return expectedSelector The selector that the hook is expected to return /// @return selector The selector that the hook actually returned function _callHook(IHooks self, bytes memory data) private returns (bytes4 expectedSelector, bytes4 selector) { - bool set = Lockers.setCurrentHook(self); - assembly { expectedSelector := mload(add(data, 0x20)) } @@ -111,9 +106,6 @@ library Hooks { if (!success) _revert(result); selector = abi.decode(result, (bytes4)); - - // We only want to clear the current hook if it was set in setCurrentHook in this execution frame. - if (set) Lockers.clearCurrentHook(); } /// @notice performs a hook call using the given calldata on the given hook diff --git a/src/libraries/Locker.sol b/src/libraries/Locker.sol new file mode 100644 index 000000000..2f45772f8 --- /dev/null +++ b/src/libraries/Locker.sol @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.20; + +import {IHooks} from "../interfaces/IHooks.sol"; + +/// @notice This is a temporary library that allows us to use transient storage (tstore/tload) +/// for the lockers array +/// TODO: This library can be deleted when we have the transient keyword support in solidity. +library Locker { + // The slot holding the locker, transiently, and the lock caller in the next slot + uint256 constant LOCKER_SLOT = uint256(keccak256("Locker")) - 1; + + /// @notice Thrown when trying to set the lock target as address(0) + /// we use locker==address(0) to signal that the pool is not locked + error InvalidLocker(); + + function setLocker(address locker) internal { + if (locker == address(0)) revert InvalidLocker(); + uint256 slot = LOCKER_SLOT; + + assembly { + // set the locker + tstore(slot, locker) + } + } + + function clearLocker() internal { + uint256 slot = LOCKER_SLOT; + assembly { + tstore(slot, 0) + } + } + + function getLocker() internal view returns (address locker) { + uint256 slot = LOCKER_SLOT; + assembly { + locker := tload(slot) + } + } + + function isLocked() internal view returns (bool) { + return getLocker() != address(0); + } +} diff --git a/src/libraries/Lockers.sol b/src/libraries/Lockers.sol deleted file mode 100644 index dd78d18b4..000000000 --- a/src/libraries/Lockers.sol +++ /dev/null @@ -1,153 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity ^0.8.24; - -import {IHooks} from "../interfaces/IHooks.sol"; - -/// @notice This is a temporary library that allows us to use transient storage (tstore/tload) -/// for the lockers array and nonzero delta count. -/// TODO: This library can be deleted when we have the transient keyword support in solidity. -library Lockers { - // The starting slot for an array of lockers, stored transiently. - uint256 constant LOCKERS_SLOT = uint256(keccak256("Lockers")) - 1; - - // The number of slots per item in the lockers array - uint256 constant LOCKER_STRUCT_SIZE = 2; - - // The starting slot for an array of hook addresses per locker, stored transiently. - uint256 constant HOOK_ADDRESS_SLOT = uint256(keccak256("HookAddress")) - 1; - - // The slot holding the number of nonzero deltas. - uint256 constant NONZERO_DELTA_COUNT = uint256(keccak256("NonzeroDeltaCount")) - 1; - - // pushes an address tuple (address locker, address lockCaller) - // to the locker array, so each length of the array represents 2 slots of tstorage - function push(address locker, address lockCaller) internal { - uint256 slot = LOCKERS_SLOT; - - uint256 newLength; - uint256 thisLockerSlot; - - unchecked { - newLength = length() + 1; - thisLockerSlot = LOCKERS_SLOT + (newLength * LOCKER_STRUCT_SIZE); - } - - assembly { - // add the locker - tstore(thisLockerSlot, locker) - - // add the lock caller - tstore(add(thisLockerSlot, 1), lockCaller) - - // increase the length - tstore(slot, newLength) - } - } - - function pop() internal { - // decrease the length - uint256 slot = LOCKERS_SLOT; - uint256 newLength; - unchecked { - newLength = length() - 1; - } - assembly { - tstore(slot, newLength) - } - } - - function length() internal view returns (uint256 _length) { - uint256 slot = LOCKERS_SLOT; - assembly { - _length := tload(slot) - } - } - - function clear() internal { - uint256 slot = LOCKERS_SLOT; - assembly { - tstore(slot, 0) - } - } - - function getLocker(uint256 i) internal view returns (address locker) { - // first slot of the ith array item - uint256 slot = LOCKERS_SLOT + (i * LOCKER_STRUCT_SIZE); - assembly { - locker := tload(slot) - } - } - - function getLockCaller(uint256 i) internal view returns (address locker) { - // second slot of the ith array item - uint256 slot = LOCKERS_SLOT + (i * LOCKER_STRUCT_SIZE + 1); - assembly { - locker := tload(slot) - } - } - - function getCurrentLocker() internal view returns (address locker) { - return getLocker(length()); - } - - function getCurrentLockCaller() internal view returns (address locker) { - return getLockCaller(length()); - } - - function nonzeroDeltaCount() internal view returns (uint256 count) { - uint256 slot = NONZERO_DELTA_COUNT; - assembly { - count := tload(slot) - } - } - - function incrementNonzeroDeltaCount() internal { - uint256 slot = NONZERO_DELTA_COUNT; - assembly { - let count := tload(slot) - count := add(count, 1) - tstore(slot, count) - } - } - - /// @notice Potential to underflow. - /// Current usage ensures this will not happen because we call decrememnt with known boundaries (only up to the numer of times we call increment). - function decrementNonzeroDeltaCount() internal { - uint256 slot = NONZERO_DELTA_COUNT; - assembly { - let count := tload(slot) - count := sub(count, 1) - tstore(slot, count) - } - } - - function getCurrentHook() internal view returns (IHooks currentHook) { - return IHooks(getHook(length())); - } - - function getHook(uint256 i) internal view returns (address hook) { - uint256 slot = HOOK_ADDRESS_SLOT + i; - assembly { - hook := tload(slot) - } - } - - function setCurrentHook(IHooks currentHook) internal returns (bool set) { - // Set the hook address for the current locker if the address is 0. - // If the address is nonzero, a hook has already been set for this lock, and is not allowed to be updated or cleared at the end of the call. - if (address(getCurrentHook()) == address(0)) { - uint256 slot = HOOK_ADDRESS_SLOT + length(); - assembly { - tstore(slot, currentHook) - } - return true; - } - } - - function clearCurrentHook() internal { - uint256 slot = HOOK_ADDRESS_SLOT + length(); - assembly { - tstore(slot, 0) - } - } -} diff --git a/src/libraries/NonZeroDeltaCount.sol b/src/libraries/NonZeroDeltaCount.sol new file mode 100644 index 000000000..7e18cdd0a --- /dev/null +++ b/src/libraries/NonZeroDeltaCount.sol @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.20; + +import {IHooks} from "../interfaces/IHooks.sol"; + +/// @notice This is a temporary library that allows us to use transient storage (tstore/tload) +/// for the nonzero delta count. +/// TODO: This library can be deleted when we have the transient keyword support in solidity. +library NonZeroDeltaCount { + // The slot holding the number of nonzero deltas. + uint256 constant NONZERO_DELTA_COUNT = uint256(keccak256("NonzeroDeltaCount")) - 1; + + function read() internal view returns (uint256 count) { + uint256 slot = NONZERO_DELTA_COUNT; + assembly { + count := tload(slot) + } + } + + function increment() internal { + uint256 slot = NONZERO_DELTA_COUNT; + assembly { + let count := tload(slot) + count := add(count, 1) + tstore(slot, count) + } + } + + /// @notice Potential to underflow. + /// Current usage ensures this will not happen because we call decrement with known boundaries (only up to the number of times we call increment). + function decrement() internal { + uint256 slot = NONZERO_DELTA_COUNT; + assembly { + let count := tload(slot) + count := sub(count, 1) + tstore(slot, count) + } + } +} diff --git a/src/test/AccessLockHook.sol b/src/test/AccessLockHook.sol deleted file mode 100644 index c0461acbc..000000000 --- a/src/test/AccessLockHook.sol +++ /dev/null @@ -1,304 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.24; - -import {BaseTestHooks} from "./BaseTestHooks.sol"; -import {PoolKey} from "../types/PoolKey.sol"; -import {IPoolManager} from "../interfaces/IPoolManager.sol"; -import {IHooks} from "../interfaces/IHooks.sol"; -import {CurrencyLibrary, Currency} from "../types/Currency.sol"; -import {Hooks} from "../libraries/Hooks.sol"; -import {TickMath} from "../libraries/TickMath.sol"; -import {Test} from "forge-std/Test.sol"; -import {ILockCallback} from "../interfaces/callback/ILockCallback.sol"; -import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; -import {Constants} from "../../test/utils/Constants.sol"; -import {PoolIdLibrary} from "../types/PoolId.sol"; -import {BalanceDelta} from "../types/BalanceDelta.sol"; - -contract AccessLockHook is Test, BaseTestHooks { - using PoolIdLibrary for PoolKey; - using CurrencyLibrary for Currency; - - IPoolManager manager; - - constructor(IPoolManager _manager) { - manager = _manager; - } - - error InvalidAction(); - - enum LockAction { - Mint, - Take, - Donate, - Swap, - ModifyLiquidity, - Burn, - Settle, - Initialize, - NoOp - } - - function beforeInitialize( - address, /* sender **/ - PoolKey calldata key, - uint160, /* sqrtPriceX96 **/ - bytes calldata hookData - ) external override returns (bytes4) { - return _executeAction(key, hookData, IHooks.beforeInitialize.selector); - } - - function beforeSwap( - address, /* sender **/ - PoolKey calldata key, - IPoolManager.SwapParams calldata, /* params **/ - bytes calldata hookData - ) external override returns (bytes4) { - return _executeAction(key, hookData, IHooks.beforeSwap.selector); - } - - function beforeDonate( - address, /* sender **/ - PoolKey calldata key, - uint256, /* amount0 **/ - uint256, /* amount1 **/ - bytes calldata hookData - ) external override returns (bytes4) { - return _executeAction(key, hookData, IHooks.beforeDonate.selector); - } - - function beforeAddLiquidity( - address, /* sender **/ - PoolKey calldata key, - IPoolManager.ModifyLiquidityParams calldata, /* params **/ - bytes calldata hookData - ) external override returns (bytes4) { - return _executeAction(key, hookData, IHooks.beforeAddLiquidity.selector); - } - - function beforeRemoveLiquidity( - address, /* sender **/ - PoolKey calldata key, - IPoolManager.ModifyLiquidityParams calldata, /* params **/ - bytes calldata hookData - ) external override returns (bytes4) { - return _executeAction(key, hookData, IHooks.beforeRemoveLiquidity.selector); - } - - function _executeAction(PoolKey memory key, bytes calldata hookData, bytes4 selector) internal returns (bytes4) { - if (hookData.length == 0) { - // We have re-entered the hook or we are initializing liquidity in the pool before testing the lock actions. - return selector; - } - (uint256 amount, LockAction action) = abi.decode(hookData, (uint256, LockAction)); - - // These actions just use some hardcoded parameters. - if (action == LockAction.Mint) { - manager.mint(address(this), key.currency1.toId(), amount); - } else if (action == LockAction.Take) { - manager.take(key.currency1, address(this), amount); - } else if (action == LockAction.Donate) { - manager.donate(key, amount, amount, new bytes(0)); - } else if (action == LockAction.Swap) { - manager.swap( - key, - IPoolManager.SwapParams({ - zeroForOne: true, - amountSpecified: int256(amount), - sqrtPriceLimitX96: TickMath.MIN_SQRT_RATIO + 1 - }), - new bytes(0) - ); - } else if (action == LockAction.ModifyLiquidity) { - manager.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -60, tickUpper: 60, liquidityDelta: int256(amount)}), - new bytes(0) - ); - } else if (action == LockAction.NoOp) { - assertEq(address(manager.getCurrentHook()), address(this)); - return Hooks.NO_OP_SELECTOR; - } else if (action == LockAction.Burn) { - manager.burn(address(this), key.currency1.toId(), amount); - } else if (action == LockAction.Settle) { - manager.take(key.currency1, address(this), amount); - assertEq(MockERC20(Currency.unwrap(key.currency1)).balanceOf(address(this)), amount); - assertEq(manager.getLockNonzeroDeltaCount(), 1); - MockERC20(Currency.unwrap(key.currency1)).transfer(address(manager), amount); - manager.settle(key.currency1); - assertEq(manager.getLockNonzeroDeltaCount(), 0); - } else if (action == LockAction.Initialize) { - PoolKey memory newKey = PoolKey({ - currency0: key.currency0, - currency1: key.currency1, - fee: Constants.FEE_LOW, - tickSpacing: 60, - hooks: IHooks(address(0)) - }); - manager.initialize(newKey, Constants.SQRT_RATIO_1_2, new bytes(0)); - } else { - revert InvalidAction(); - } - - return selector; - } -} - -// Hook that can access the lock. -// Also has the ability to call out to another hook or pool. -contract AccessLockHook2 is Test, BaseTestHooks { - IPoolManager manager; - - using CurrencyLibrary for Currency; - - error IncorrectHookSet(); - - constructor(IPoolManager _manager) { - manager = _manager; - } - - function beforeAddLiquidity( - address sender, - PoolKey calldata key, - IPoolManager.ModifyLiquidityParams calldata params, - bytes calldata hookData - ) external override returns (bytes4) { - if (address(manager.getCurrentHook()) != address(this)) { - revert IncorrectHookSet(); - } - - (bool shouldCallHook, PoolKey memory key2) = abi.decode(hookData, (bool, PoolKey)); - - if (shouldCallHook) { - // Should revert. - bytes memory hookData2 = abi.encode(100, AccessLockHook.LockAction.Mint); - IHooks(key2.hooks).beforeAddLiquidity(sender, key, params, hookData2); // params dont really matter, just want to tell the other hook to do a mint action, but will revert - } else { - // Should succeed and should NOT set the current hook to key2.hooks. - // The permissions should remain to THIS hook during this lock. - manager.modifyLiquidity(key2, params, new bytes(0)); - - if (address(manager.getCurrentHook()) != address(this)) { - revert IncorrectHookSet(); - } - // Should succeed. - manager.mint(address(this), key.currency1.toId(), 10); - } - return IHooks.beforeAddLiquidity.selector; - } -} - -// Reenters the PoolManager to donate and asserts currentHook is set and unset correctly throughout the popping and pushing of locks. -contract AccessLockHook3 is Test, ILockCallback, BaseTestHooks { - IPoolManager manager; - // The pool to donate to in the nested lock. - // Ensure this has balance of currency0.abi - PoolKey key; - - constructor(IPoolManager _manager) { - manager = _manager; - } - - // Instead of passing through key all the way to the nested lock, just save it. - function setKey(PoolKey memory _key) external { - key = _key; - } - - function beforeAddLiquidity( - address, /* sender **/ - PoolKey calldata, /* key **/ - IPoolManager.ModifyLiquidityParams calldata, /* params **/ - bytes calldata /* hookData **/ - ) external override returns (bytes4) { - assertEq(address(manager.getCurrentHook()), address(this)); - manager.lock(address(this), abi.encode(true)); - assertEq(address(manager.getCurrentHook()), address(this)); - manager.lock(address(this), abi.encode(false)); - assertEq(address(manager.getCurrentHook()), address(this)); - return IHooks.beforeAddLiquidity.selector; - } - - function lockAcquired(address caller, bytes memory data) external returns (bytes memory) { - require(caller == address(this)); - assertEq(manager.getLockLength(), 2); - assertEq(address(manager.getCurrentHook()), address(0)); - - (bool isFirstLock) = abi.decode(data, (bool)); - if (isFirstLock) { - manager.donate(key, 10, 0, new bytes(0)); - assertEq(address(manager.getCurrentHook()), address(key.hooks)); - MockERC20(Currency.unwrap(key.currency0)).transfer(address(manager), 10); - manager.settle(key.currency0); - } - return data; - } -} - -contract AccessLockFeeHook is Test, BaseTestHooks { - IPoolManager manager; - - uint256 constant WITHDRAWAL_FEE_BIPS = 40; // 40/10000 = 0.4% - uint256 constant SWAP_FEE_BIPS = 55; // 55/10000 = 0.55% - uint256 constant TOTAL_BIPS = 10000; - - constructor(IPoolManager _manager) { - manager = _manager; - } - - function afterAddLiquidity( - address, /* sender **/ - PoolKey calldata key, - IPoolManager.ModifyLiquidityParams calldata, /* params **/ - BalanceDelta delta, - bytes calldata /* hookData **/ - ) external override returns (bytes4) { - int128 amount0 = delta.amount0(); - int128 amount1 = delta.amount1(); - - // positive delta => user owes money => liquidity addition - assert(amount0 >= 0 && amount1 >= 0); - - return IHooks.afterAddLiquidity.selector; - } - - function afterRemoveLiquidity( - address, /* sender **/ - PoolKey calldata key, - IPoolManager.ModifyLiquidityParams calldata, /* params **/ - BalanceDelta delta, - bytes calldata /* hookData **/ - ) external override returns (bytes4) { - int128 amount0 = delta.amount0(); - int128 amount1 = delta.amount1(); - - // negative delta => user is owed money => liquidity withdrawal - uint256 amount0Fee = uint128(-amount0) * WITHDRAWAL_FEE_BIPS / TOTAL_BIPS; - uint256 amount1Fee = uint128(-amount1) * WITHDRAWAL_FEE_BIPS / TOTAL_BIPS; - - manager.take(key.currency0, address(this), amount0Fee); - manager.take(key.currency1, address(this), amount1Fee); - - return IHooks.afterRemoveLiquidity.selector; - } - - function afterSwap( - address, /* sender **/ - PoolKey calldata key, - IPoolManager.SwapParams calldata params, - BalanceDelta delta, - bytes calldata /* hookData **/ - ) external override returns (bytes4) { - int128 amount0 = delta.amount0(); - int128 amount1 = delta.amount1(); - - // fee on output token - output delta will be negative - (Currency feeCurrency, uint256 outputAmount) = - (params.zeroForOne) ? (key.currency1, uint128(-amount1)) : (key.currency0, uint128(-amount0)); - - uint256 feeAmount = outputAmount * SWAP_FEE_BIPS / TOTAL_BIPS; - - manager.take(feeCurrency, address(this), feeAmount); - - return IHooks.afterSwap.selector; - } -} diff --git a/src/test/EmptyTestHooks.sol b/src/test/EmptyTestHooks.sol index 631a98c53..117312a85 100644 --- a/src/test/EmptyTestHooks.sol +++ b/src/test/EmptyTestHooks.sol @@ -23,8 +23,7 @@ contract EmptyTestHooks is IHooks { afterSwap: true, beforeDonate: true, afterDonate: true, - noOp: true, - accessLock: true + noOp: true }) ); } diff --git a/src/test/NoOpTestHooks.sol b/src/test/NoOpTestHooks.sol index 7007ef990..b9c10ebac 100644 --- a/src/test/NoOpTestHooks.sol +++ b/src/test/NoOpTestHooks.sol @@ -22,8 +22,7 @@ contract NoOpTestHooks is BaseTestHooks { afterSwap: false, beforeDonate: true, afterDonate: false, - noOp: true, - accessLock: false + noOp: true }) ); } diff --git a/src/test/PoolDonateTest.sol b/src/test/PoolDonateTest.sol index 164f58085..7ee393db2 100644 --- a/src/test/PoolDonateTest.sol +++ b/src/test/PoolDonateTest.sol @@ -45,27 +45,24 @@ contract PoolDonateTest is PoolTestBase, Test { CallbackData memory data = abi.decode(rawData, (CallbackData)); - (,, uint256 reserveBefore0, int256 deltaBefore0) = _fetchBalances(data.key.currency0, data.sender); - (,, uint256 reserveBefore1, int256 deltaBefore1) = _fetchBalances(data.key.currency1, data.sender); + (,, uint256 reserveBefore0, int256 deltaBefore0) = + _fetchBalances(data.key.currency0, data.sender, address(this)); + (,, uint256 reserveBefore1, int256 deltaBefore1) = + _fetchBalances(data.key.currency1, data.sender, address(this)); assertEq(deltaBefore0, 0); assertEq(deltaBefore1, 0); BalanceDelta delta = manager.donate(data.key, data.amount0, data.amount1, data.hookData); - // Checks that the current hook is cleared if there is an access lock. Note that if this router is ever used in a nested lock this will fail. - assertEq(address(manager.getCurrentHook()), address(0)); + (,, uint256 reserveAfter0, int256 deltaAfter0) = _fetchBalances(data.key.currency0, data.sender, address(this)); + (,, uint256 reserveAfter1, int256 deltaAfter1) = _fetchBalances(data.key.currency1, data.sender, address(this)); - (,, uint256 reserveAfter0, int256 deltaAfter0) = _fetchBalances(data.key.currency0, data.sender); - (,, uint256 reserveAfter1, int256 deltaAfter1) = _fetchBalances(data.key.currency1, data.sender); - - if (!data.key.hooks.hasPermission(Hooks.ACCESS_LOCK_FLAG)) { - assertEq(reserveBefore0, reserveAfter0); - assertEq(reserveBefore1, reserveAfter1); - if (!data.key.hooks.hasPermission(Hooks.NO_OP_FLAG)) { - assertEq(deltaAfter0, int256(data.amount0)); - assertEq(deltaAfter1, int256(data.amount1)); - } + assertEq(reserveBefore0, reserveAfter0); + assertEq(reserveBefore1, reserveAfter1); + if (!data.key.hooks.hasPermission(Hooks.NO_OP_FLAG)) { + assertEq(deltaAfter0, int256(data.amount0)); + assertEq(deltaAfter1, int256(data.amount1)); } if (delta == BalanceDeltaLibrary.MAXIMUM_DELTA) { diff --git a/src/test/PoolLockTest.sol b/src/test/PoolEmptyLockTest.sol similarity index 93% rename from src/test/PoolLockTest.sol rename to src/test/PoolEmptyLockTest.sol index b3dbc8cde..05b59e30a 100644 --- a/src/test/PoolLockTest.sol +++ b/src/test/PoolEmptyLockTest.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.24; import {IPoolManager} from "../interfaces/IPoolManager.sol"; import {ILockCallback} from "../interfaces/callback/ILockCallback.sol"; -contract PoolLockTest is ILockCallback { +contract PoolEmptyLockTest is ILockCallback { event LockAcquired(); IPoolManager manager; diff --git a/src/test/PoolInitializeTest.sol b/src/test/PoolInitializeTest.sol index 6a0a03857..b7bf28c85 100644 --- a/src/test/PoolInitializeTest.sol +++ b/src/test/PoolInitializeTest.sol @@ -44,17 +44,9 @@ contract PoolInitializeTest is Test, PoolTestBase { int256 delta1 = manager.currencyDelta(address(this), data.key.currency1); uint256 nonZeroDC = manager.getLockNonzeroDeltaCount(); - if (!data.key.hooks.hasPermission(Hooks.ACCESS_LOCK_FLAG)) { - assertEq(delta0, 0, "delta0"); - assertEq(delta1, 0, "delta1"); - assertEq(nonZeroDC, 0, "NonzeroDeltaCount"); - } else { - // settle deltas - if (delta0 > 0) _settle(data.key.currency0, data.sender, int128(delta0), true); - if (delta1 > 0) _settle(data.key.currency1, data.sender, int128(delta1), true); - if (delta0 < 0) _take(data.key.currency0, data.sender, int128(delta0), true); - if (delta1 < 0) _take(data.key.currency1, data.sender, int128(delta1), true); - } + assertEq(delta0, 0, "delta0"); + assertEq(delta1, 0, "delta1"); + assertEq(nonZeroDC, 0, "NonzeroDeltaCount"); return abi.encode(tick); } diff --git a/src/test/PoolModifyLiquidityTest.sol b/src/test/PoolModifyLiquidityTest.sol index 21412ef87..3cf7a924f 100644 --- a/src/test/PoolModifyLiquidityTest.sol +++ b/src/test/PoolModifyLiquidityTest.sol @@ -62,21 +62,16 @@ contract PoolModifyLiquidityTest is Test, PoolTestBase { CallbackData memory data = abi.decode(rawData, (CallbackData)); BalanceDelta delta = manager.modifyLiquidity(data.key, data.params, data.hookData); - // Checks that the current hook is cleared if there is an access lock. Note that if this router is ever used in a nested lock this will fail. - assertEq(address(manager.getCurrentHook()), address(0)); - (,,, int256 delta0) = _fetchBalances(data.key.currency0, data.sender); - (,,, int256 delta1) = _fetchBalances(data.key.currency1, data.sender); + (,,, int256 delta0) = _fetchBalances(data.key.currency0, data.sender, address(this)); + (,,, int256 delta1) = _fetchBalances(data.key.currency1, data.sender, address(this)); - // These assertions only apply in non lock-accessing pools. - if (!data.key.hooks.hasPermission(Hooks.ACCESS_LOCK_FLAG)) { - if (data.params.liquidityDelta > 0) { - assert(delta0 > 0 || delta1 > 0 || data.key.hooks.hasPermission(Hooks.NO_OP_FLAG)); - assert(!(delta0 < 0 || delta1 < 0)); - } else { - assert(delta0 < 0 || delta1 < 0 || data.key.hooks.hasPermission(Hooks.NO_OP_FLAG)); - assert(!(delta0 > 0 || delta1 > 0)); - } + if (data.params.liquidityDelta > 0) { + assert(delta0 > 0 || delta1 > 0 || data.key.hooks.hasPermission(Hooks.NO_OP_FLAG)); + assert(!(delta0 < 0 || delta1 < 0)); + } else { + assert(delta0 < 0 || delta1 < 0 || data.key.hooks.hasPermission(Hooks.NO_OP_FLAG)); + assert(!(delta0 > 0 || delta1 > 0)); } if (delta0 > 0) _settle(data.key.currency0, data.sender, int128(delta0), data.settleUsingTransfer); diff --git a/src/test/PoolNestedActionsTest.sol b/src/test/PoolNestedActionsTest.sol new file mode 100644 index 000000000..e792e3e49 --- /dev/null +++ b/src/test/PoolNestedActionsTest.sol @@ -0,0 +1,214 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.20; + +import {IPoolManager} from "../interfaces/IPoolManager.sol"; +import {ILockCallback} from "../interfaces/callback/ILockCallback.sol"; +import {PoolTestBase} from "./PoolTestBase.sol"; +import {PoolKey} from "../types/PoolKey.sol"; +import {Constants} from "../../test/utils/Constants.sol"; +import {Test} from "forge-std/Test.sol"; +import {BalanceDelta} from "../types/BalanceDelta.sol"; +import {Currency} from "../types/Currency.sol"; + +enum Action { + NESTED_SELF_LOCK, + NESTED_EXECUTOR_LOCK, + SWAP_AND_SETTLE, + DONATE_AND_SETTLE, + ADD_LIQ_AND_SETTLE, + REMOVE_LIQ_AND_SETTLE +} + +contract PoolNestedActionsTest is Test, ILockCallback { + IPoolManager manager; + NestedActionExecutor public executor; + address user; + + constructor(IPoolManager _manager) { + manager = _manager; + user = msg.sender; + executor = new NestedActionExecutor(manager, user); + } + + /// @notice Called by the pool manager on `msg.sender` when a lock is acquired + function lockAcquired(address, bytes calldata data) external override returns (bytes memory) { + Action[] memory actions = abi.decode(data, (Action[])); + if (actions.length == 1 && actions[0] == Action.NESTED_SELF_LOCK) { + _nestedLock(); + } else { + executor.execute(actions); + } + return ""; + } + + function _nestedLock() internal { + address locker = manager.getLocker(); + assertEq(locker, address(this)); + + vm.expectRevert(abi.encodeWithSelector(IPoolManager.AlreadyLocked.selector)); + manager.lock(address(this), ""); + + locker = manager.getLocker(); + assertEq(locker, address(this)); + } +} + +contract NestedActionExecutor is Test, PoolTestBase { + PoolKey internal key; + address user; + + error KeyNotSet(); + + IPoolManager.ModifyLiquidityParams internal ADD_LIQ_PARAMS = + IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1e18}); + + IPoolManager.ModifyLiquidityParams internal REMOVE_LIQ_PARAMS = + IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: -1e18}); + + IPoolManager.SwapParams internal SWAP_PARAMS = + IPoolManager.SwapParams({zeroForOne: true, amountSpecified: 100, sqrtPriceLimitX96: Constants.SQRT_RATIO_1_2}); + + uint256 internal DONATE_AMOUNT0 = 12345e6; + uint256 internal DONATE_AMOUNT1 = 98765e4; + + constructor(IPoolManager _manager, address _user) PoolTestBase(_manager) { + user = _user; + } + + function setKey(PoolKey memory _key) external { + key = _key; + } + + function execute(Action[] memory actions) public { + if (Currency.unwrap(key.currency0) == address(0)) revert KeyNotSet(); + for (uint256 i = 0; i < actions.length; i++) { + Action action = actions[i]; + if (action == Action.NESTED_EXECUTOR_LOCK) _nestedLock(); + else if (action == Action.SWAP_AND_SETTLE) _swap(); + else if (action == Action.ADD_LIQ_AND_SETTLE) _addLiquidity(); + else if (action == Action.REMOVE_LIQ_AND_SETTLE) _removeLiquidity(); + else if (action == Action.DONATE_AND_SETTLE) _donate(); + } + } + + function _nestedLock() internal { + (address locker) = manager.getLocker(); + assertEq(locker, msg.sender); + + vm.expectRevert(abi.encodeWithSelector(IPoolManager.AlreadyLocked.selector)); + manager.lock(address(this), ""); + + (locker) = manager.getLocker(); + assertEq(locker, msg.sender); + } + + function _swap() internal { + (address locker) = manager.getLocker(); + assertTrue(locker != address(this), "Locker wrong"); + + (,,, int256 deltaLockerBefore0) = _fetchBalances(key.currency0, user, locker); + (,,, int256 deltaLockerBefore1) = _fetchBalances(key.currency1, user, locker); + (,,, int256 deltaThisBefore0) = _fetchBalances(key.currency0, user, address(this)); + (,,, int256 deltaThisBefore1) = _fetchBalances(key.currency1, user, address(this)); + + BalanceDelta delta = manager.swap(key, SWAP_PARAMS, ""); + + (,,, int256 deltaLockerAfter0) = _fetchBalances(key.currency0, user, locker); + (,,, int256 deltaLockerAfter1) = _fetchBalances(key.currency1, user, locker); + (,,, int256 deltaThisAfter0) = _fetchBalances(key.currency0, user, address(this)); + (,,, int256 deltaThisAfter1) = _fetchBalances(key.currency1, user, address(this)); + + assertEq(deltaLockerBefore0, deltaLockerAfter0, "Locker delta 0"); + assertEq(deltaLockerBefore1, deltaLockerAfter1, "Locker delta 1"); + assertEq(deltaThisBefore0 + SWAP_PARAMS.amountSpecified, deltaThisAfter0, "Executor delta 0"); + assertEq(deltaThisBefore1 - 98, deltaThisAfter1, "Executor delta 1"); + assertEq(delta.amount0(), deltaThisAfter0, "Swap delta 0"); + assertEq(delta.amount1(), deltaThisAfter1, "Swap delta 1"); + + _settle(key.currency0, user, int128(deltaThisAfter0), true); + _take(key.currency1, user, int128(deltaThisAfter1), true); + } + + function _addLiquidity() internal { + address locker = manager.getLocker(); + assertTrue(locker != address(this), "Locker wrong"); + + (,,, int256 deltaLockerBefore0) = _fetchBalances(key.currency0, user, locker); + (,,, int256 deltaLockerBefore1) = _fetchBalances(key.currency1, user, locker); + (,,, int256 deltaThisBefore0) = _fetchBalances(key.currency0, user, address(this)); + (,,, int256 deltaThisBefore1) = _fetchBalances(key.currency1, user, address(this)); + + BalanceDelta delta = manager.modifyLiquidity(key, ADD_LIQ_PARAMS, ""); + + (,,, int256 deltaLockerAfter0) = _fetchBalances(key.currency0, user, locker); + (,,, int256 deltaLockerAfter1) = _fetchBalances(key.currency1, user, locker); + (,,, int256 deltaThisAfter0) = _fetchBalances(key.currency0, user, address(this)); + (,,, int256 deltaThisAfter1) = _fetchBalances(key.currency1, user, address(this)); + + assertEq(deltaLockerBefore0, deltaLockerAfter0, "Locker delta 0"); + assertEq(deltaLockerBefore1, deltaLockerAfter1, "Locker delta 1"); + assertEq(deltaThisBefore0 + delta.amount0(), deltaThisAfter0, "Executor delta 0"); + assertEq(deltaThisBefore1 + delta.amount1(), deltaThisAfter1, "Executor delta 1"); + + _settle(key.currency0, user, int128(deltaThisAfter0), true); + _settle(key.currency1, user, int128(deltaThisAfter1), true); + } + + // cannot remove non-existent liquidity - need to perform an add before this removal + function _removeLiquidity() internal { + address locker = manager.getLocker(); + assertTrue(locker != address(this), "Locker wrong"); + + (,,, int256 deltaLockerBefore0) = _fetchBalances(key.currency0, user, locker); + (,,, int256 deltaLockerBefore1) = _fetchBalances(key.currency1, user, locker); + (,,, int256 deltaThisBefore0) = _fetchBalances(key.currency0, user, address(this)); + (,,, int256 deltaThisBefore1) = _fetchBalances(key.currency1, user, address(this)); + + BalanceDelta delta = manager.modifyLiquidity(key, REMOVE_LIQ_PARAMS, ""); + + (,,, int256 deltaLockerAfter0) = _fetchBalances(key.currency0, user, locker); + (,,, int256 deltaLockerAfter1) = _fetchBalances(key.currency1, user, locker); + (,,, int256 deltaThisAfter0) = _fetchBalances(key.currency0, user, address(this)); + (,,, int256 deltaThisAfter1) = _fetchBalances(key.currency1, user, address(this)); + + assertEq(deltaLockerBefore0, deltaLockerAfter0, "Locker delta 0"); + assertEq(deltaLockerBefore1, deltaLockerAfter1, "Locker delta 1"); + assertEq(deltaThisBefore0 + delta.amount0(), deltaThisAfter0, "Executor delta 0"); + assertEq(deltaThisBefore1 + delta.amount1(), deltaThisAfter1, "Executor delta 1"); + + _take(key.currency0, user, int128(deltaThisAfter0), true); + _take(key.currency1, user, int128(deltaThisAfter1), true); + } + + function _donate() internal { + address locker = manager.getLocker(); + assertTrue(locker != address(this), "Locker wrong"); + + (,,, int256 deltaLockerBefore0) = _fetchBalances(key.currency0, user, locker); + (,,, int256 deltaLockerBefore1) = _fetchBalances(key.currency1, user, locker); + (,,, int256 deltaThisBefore0) = _fetchBalances(key.currency0, user, address(this)); + (,,, int256 deltaThisBefore1) = _fetchBalances(key.currency1, user, address(this)); + + BalanceDelta delta = manager.donate(key, DONATE_AMOUNT0, DONATE_AMOUNT1, ""); + + (,,, int256 deltaLockerAfter0) = _fetchBalances(key.currency0, user, locker); + (,,, int256 deltaLockerAfter1) = _fetchBalances(key.currency1, user, locker); + (,,, int256 deltaThisAfter0) = _fetchBalances(key.currency0, user, address(this)); + (,,, int256 deltaThisAfter1) = _fetchBalances(key.currency1, user, address(this)); + + assertEq(deltaLockerBefore0, deltaLockerAfter0, "Locker delta 0"); + assertEq(deltaLockerBefore1, deltaLockerAfter1, "Locker delta 1"); + assertEq(deltaThisBefore0 + int256(DONATE_AMOUNT0), deltaThisAfter0, "Executor delta 0"); + assertEq(deltaThisBefore1 + int256(DONATE_AMOUNT1), deltaThisAfter1, "Executor delta 1"); + assertEq(delta.amount0(), int256(DONATE_AMOUNT0), "Donate delta 0"); + assertEq(delta.amount1(), int256(DONATE_AMOUNT1), "Donate delta 1"); + + _settle(key.currency0, user, int128(deltaThisAfter0), true); + _settle(key.currency1, user, int128(deltaThisAfter1), true); + } + + // This will never actually be used - its just to allow us to use the PoolTestBase helper contact + function lockAcquired(address, bytes calldata) external pure override returns (bytes memory) { + return ""; + } +} diff --git a/src/test/PoolSwapTest.sol b/src/test/PoolSwapTest.sol index a4e1e5742..5cde518f6 100644 --- a/src/test/PoolSwapTest.sol +++ b/src/test/PoolSwapTest.sol @@ -54,47 +54,42 @@ contract PoolSwapTest is Test, PoolTestBase { CallbackData memory data = abi.decode(rawData, (CallbackData)); - (,, uint256 reserveBefore0, int256 deltaBefore0) = _fetchBalances(data.key.currency0, data.sender); - (,, uint256 reserveBefore1, int256 deltaBefore1) = _fetchBalances(data.key.currency1, data.sender); + (,, uint256 reserveBefore0, int256 deltaBefore0) = + _fetchBalances(data.key.currency0, data.sender, address(this)); + (,, uint256 reserveBefore1, int256 deltaBefore1) = + _fetchBalances(data.key.currency1, data.sender, address(this)); assertEq(deltaBefore0, 0); assertEq(deltaBefore1, 0); BalanceDelta delta = manager.swap(data.key, data.params, data.hookData); - // Checks that the current hook is cleared if there is an access lock. Note that if this router is ever used in a nested lock this will fail. - assertEq(address(manager.getCurrentHook()), address(0)); - - (,, uint256 reserveAfter0, int256 deltaAfter0) = _fetchBalances(data.key.currency0, data.sender); - (,, uint256 reserveAfter1, int256 deltaAfter1) = _fetchBalances(data.key.currency1, data.sender); - - if (!data.key.hooks.hasPermission(Hooks.ACCESS_LOCK_FLAG)) { - // Hanndle assertions when the hook cannot access the lock. - // IE if the hook can access the lock, the reserves before and after are not necessarily the same. Hook can "take". - assertEq(reserveBefore0, reserveAfter0); - assertEq(reserveBefore1, reserveAfter1); - - if (!data.key.hooks.hasPermission(Hooks.NO_OP_FLAG)) { - if (data.params.zeroForOne) { - if (data.params.amountSpecified > 0) { - // exact input, 0 for 1 - assertEq(deltaAfter0, data.params.amountSpecified); - assert(deltaAfter1 < 0); - } else { - // exact output, 0 for 1 - assert(deltaAfter0 > 0); - assertEq(deltaAfter1, data.params.amountSpecified); - } + (,, uint256 reserveAfter0, int256 deltaAfter0) = _fetchBalances(data.key.currency0, data.sender, address(this)); + (,, uint256 reserveAfter1, int256 deltaAfter1) = _fetchBalances(data.key.currency1, data.sender, address(this)); + + assertEq(reserveBefore0, reserveAfter0); + assertEq(reserveBefore1, reserveAfter1); + + if (!data.key.hooks.hasPermission(Hooks.NO_OP_FLAG)) { + if (data.params.zeroForOne) { + if (data.params.amountSpecified > 0) { + // exact input, 0 for 1 + assertEq(deltaAfter0, data.params.amountSpecified); + assert(deltaAfter1 < 0); + } else { + // exact output, 0 for 1 + assert(deltaAfter0 > 0); + assertEq(deltaAfter1, data.params.amountSpecified); + } + } else { + if (data.params.amountSpecified > 0) { + // exact input, 1 for 0 + assertEq(deltaAfter1, data.params.amountSpecified); + assert(deltaAfter0 < 0); } else { - if (data.params.amountSpecified > 0) { - // exact input, 1 for 0 - assertEq(deltaAfter1, data.params.amountSpecified); - assert(deltaAfter0 < 0); - } else { - // exact output, 1 for 0 - assert(deltaAfter1 > 0); - assertEq(deltaAfter0, data.params.amountSpecified); - } + // exact output, 1 for 0 + assert(deltaAfter1 > 0); + assertEq(deltaAfter0, data.params.amountSpecified); } } } diff --git a/src/test/PoolTakeTest.sol b/src/test/PoolTakeTest.sol index 4f5e9e0cc..5a1cfdf9b 100644 --- a/src/test/PoolTakeTest.sol +++ b/src/test/PoolTakeTest.sol @@ -38,13 +38,13 @@ contract PoolTakeTest is Test, PoolTestBase { function _testTake(Currency currency, address sender, uint256 amount) internal { (uint256 userBalBefore, uint256 pmBalBefore, uint256 reserveBefore, int256 deltaBefore) = - _fetchBalances(currency, sender); + _fetchBalances(currency, sender, address(this)); assertEq(deltaBefore, 0); _take(currency, sender, -(amount.toInt128()), true); (uint256 userBalAfter, uint256 pmBalAfter, uint256 reserveAfter, int256 deltaAfter) = - _fetchBalances(currency, sender); + _fetchBalances(currency, sender, address(this)); assertEq(deltaAfter, amount.toInt128()); assertEq(userBalAfter - userBalBefore, amount); diff --git a/src/test/PoolTestBase.sol b/src/test/PoolTestBase.sol index bbab3bef0..020934ba6 100644 --- a/src/test/PoolTestBase.sol +++ b/src/test/PoolTestBase.sol @@ -39,7 +39,7 @@ abstract contract PoolTestBase is ILockCallback { } } - function _fetchBalances(Currency currency, address user) + function _fetchBalances(Currency currency, address user, address deltaHolder) internal view returns (uint256 userBalance, uint256 poolBalance, uint256 reserves, int256 delta) @@ -47,6 +47,6 @@ abstract contract PoolTestBase is ILockCallback { userBalance = currency.balanceOf(user); poolBalance = currency.balanceOf(address(manager)); reserves = manager.reservesOf(currency); - delta = manager.currencyDelta(address(this), currency); + delta = manager.currencyDelta(deltaHolder, currency); } } diff --git a/test/AccessLock.t.sol b/test/AccessLock.t.sol deleted file mode 100644 index 3ccc6a2bb..000000000 --- a/test/AccessLock.t.sol +++ /dev/null @@ -1,901 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.20; - -import {Test} from "forge-std/Test.sol"; -import {AccessLockHook, AccessLockHook2, AccessLockHook3, AccessLockFeeHook} from "../src/test/AccessLockHook.sol"; -import {IPoolManager} from "../src/interfaces/IPoolManager.sol"; -import {PoolModifyLiquidityTest} from "../src/test/PoolModifyLiquidityTest.sol"; -import {PoolSwapTest} from "../src/test/PoolSwapTest.sol"; -import {PoolDonateTest} from "../src/test/PoolDonateTest.sol"; -import {Constants} from "./utils/Constants.sol"; -import {PoolKey} from "../src/types/PoolKey.sol"; -import {Deployers} from "./utils/Deployers.sol"; -import {Currency, CurrencyLibrary} from "../src/types/Currency.sol"; -import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; -import {Hooks} from "../src/libraries/Hooks.sol"; -import {IHooks} from "../src/interfaces/IHooks.sol"; -import {BalanceDelta, BalanceDeltaLibrary} from "../src/types/BalanceDelta.sol"; -import {Pool} from "../src/libraries/Pool.sol"; -import {TickMath} from "../src/libraries/TickMath.sol"; -import {PoolIdLibrary} from "../src/types/PoolId.sol"; - -contract AccessLockTest is Test, Deployers { - using Pool for Pool.State; - using CurrencyLibrary for Currency; - using PoolIdLibrary for PoolKey; - using BalanceDeltaLibrary for BalanceDelta; - - AccessLockHook accessLockHook; - AccessLockHook noAccessLockHook; - AccessLockHook2 accessLockHook2; - AccessLockHook3 accessLockHook3; - AccessLockHook accessLockNoOpHook; - AccessLockFeeHook accessLockFeeHook; - - // global for stack too deep errors - BalanceDelta delta; - - uint128 amount = 1e18; - - function setUp() public { - // Initialize managers and routers. - deployFreshManagerAndRouters(); - (currency0, currency1) = deployMintAndApprove2Currencies(); - - // Create AccessLockHook. - address accessLockAddress = address( - uint160( - Hooks.ACCESS_LOCK_FLAG | Hooks.BEFORE_SWAP_FLAG | Hooks.BEFORE_ADD_LIQUIDITY_FLAG - | Hooks.BEFORE_DONATE_FLAG | Hooks.BEFORE_REMOVE_LIQUIDITY_FLAG - ) - ); - deployCodeTo("AccessLockHook.sol:AccessLockHook", abi.encode(manager), accessLockAddress); - accessLockHook = AccessLockHook(accessLockAddress); - - (key,) = - initPool(currency0, currency1, IHooks(accessLockAddress), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES); - - // Create AccessLockHook2. - address accessLockAddress2 = address(uint160(Hooks.ACCESS_LOCK_FLAG | Hooks.BEFORE_ADD_LIQUIDITY_FLAG)); - deployCodeTo("AccessLockHook.sol:AccessLockHook2", abi.encode(manager), accessLockAddress2); - accessLockHook2 = AccessLockHook2(accessLockAddress2); - - // Create AccessLockHook3. - address accessLockAddress3 = address( - (uint160(makeAddr("hook3")) << 10) >> 10 - | (uint160(Hooks.ACCESS_LOCK_FLAG | Hooks.BEFORE_ADD_LIQUIDITY_FLAG)) - ); - deployCodeTo("AccessLockHook.sol:AccessLockHook3", abi.encode(manager), accessLockAddress3); - accessLockHook3 = AccessLockHook3(accessLockAddress3); - - // Create NoAccessLockHook. - address noAccessLockHookAddress = address(uint160(Hooks.BEFORE_ADD_LIQUIDITY_FLAG)); - deployCodeTo("AccessLockHook.sol:AccessLockHook", abi.encode(manager), noAccessLockHookAddress); - noAccessLockHook = AccessLockHook(noAccessLockHookAddress); - - // Create AccessLockHook with NoOp. - address accessLockNoOpHookAddress = address( - uint160( - Hooks.NO_OP_FLAG | Hooks.ACCESS_LOCK_FLAG | Hooks.BEFORE_INITIALIZE_FLAG | Hooks.BEFORE_SWAP_FLAG - | Hooks.BEFORE_ADD_LIQUIDITY_FLAG | Hooks.BEFORE_DONATE_FLAG - ) - ); - deployCodeTo("AccessLockHook.sol:AccessLockHook", abi.encode(manager), accessLockNoOpHookAddress); - accessLockNoOpHook = AccessLockHook(accessLockNoOpHookAddress); - - // Create AccessLockFeeHook - address accessLockFeeHookAddress = address( - uint160( - Hooks.ACCESS_LOCK_FLAG | Hooks.AFTER_SWAP_FLAG | Hooks.AFTER_ADD_LIQUIDITY_FLAG - | Hooks.AFTER_REMOVE_LIQUIDITY_FLAG - ) - ); - deployCodeTo("AccessLockHook.sol:AccessLockFeeHook", abi.encode(manager), accessLockFeeHookAddress); - accessLockFeeHook = AccessLockFeeHook(accessLockFeeHookAddress); - } - - function test_onlyByLocker_revertsForNoAccessLockPool() public { - (PoolKey memory keyWithoutAccessLockFlag,) = - initPool(currency0, currency1, IHooks(noAccessLockHook), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES); - - vm.expectRevert( - abi.encodeWithSelector( - IPoolManager.LockedBy.selector, address(modifyLiquidityRouter), address(noAccessLockHook) - ) - ); - modifyLiquidityRouter.modifyLiquidity( - keyWithoutAccessLockFlag, - IPoolManager.ModifyLiquidityParams({tickLower: 0, tickUpper: 60, liquidityDelta: 1e18}), - abi.encode(10, AccessLockHook.LockAction.Mint) // attempts a mint action that should revert - ); - } - /** - * - * - * The following test suite tests that appropriate hooks can call - * every function gated by the `onlyByLocker` modifier. - * We call these "LockActions". - * LockActions: - * - Mint - * - Take - * - Swap - * - ModifyLiquidity - * - Donate - * - Burn - * - Settle - * - Initialize - * Each of these calls is then tested from every callback after the - * currentHook gets set (beforeAddLiquidity, beforeSwap, and beforeDonate). - * - */ - - /** - * - * BEFORE MODIFY POSITION TESTS - * - */ - function test_beforeAddLiquidity_mint_succeedsWithAccessLock() public { - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - delta = modifyLiquidityRouter.modifyLiquidity( - key, IPoolManager.ModifyLiquidityParams(0, 60, 1e18), abi.encode(amount, AccessLockHook.LockAction.Mint) - ); - - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - assertEq(balanceOfBefore0 - balanceOfAfter0, uint256(uint128(delta.amount0()))); - // The balance of our contract should be from the modifyLiquidityRouter (delta) AND the hook (amount). - assertEq(balanceOfBefore1 - balanceOfAfter1, uint256(amount + uint256(uint128(delta.amount1())))); - - assertEq(manager.balanceOf(address(accessLockHook), currency1.toId()), amount); - } - - function test_beforeRemoveLiquidity_mint_succeedsWithAccessLock() public { - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - modifyLiquidityRouter.modifyLiquidity(key, LIQ_PARAMS, ZERO_BYTES); - modifyLiquidityRouter.modifyLiquidity( - key, REMOVE_LIQ_PARAMS, abi.encode(amount, AccessLockHook.LockAction.Mint) - ); - - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - // The balance of our contract should be equal to our original balance because we added and removed our liquidity. - // Note: the balance is off by one and is a known issue documented here: https://github.com/Uniswap/v3-core/issues/570 - assertTrue(balanceOfBefore0 - balanceOfAfter0 <= 1); - assertTrue(balanceOfBefore1 - balanceOfAfter1 - amount <= 1); - - assertEq(manager.balanceOf(address(accessLockHook), currency1.toId()), amount); - } - - function test_beforeAddLiquidity_take_succeedsWithAccessLock() public { - // Add liquidity so there is something to take. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - // Hook only takes currency 1 rn. - delta = modifyLiquidityRouter.modifyLiquidity( - key, IPoolManager.ModifyLiquidityParams(-60, 60, 1e18), abi.encode(amount, AccessLockHook.LockAction.Take) - ); - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - assertEq(balanceOfBefore0 - balanceOfAfter0, uint256(uint128(delta.amount0()))); - // The balance of our contract should be from the modifyLiquidityRouter (delta) AND the hook (amount). - assertEq(balanceOfBefore1 - balanceOfAfter1, uint256(amount + uint256(uint128(delta.amount1())))); - assertEq(MockERC20(Currency.unwrap(currency1)).balanceOf(address(accessLockHook)), amount); - } - - function test_beforeRemoveLiquidity_take_succeedsWithAccessLock() public { - // Add liquidity so there is something to take. - delta = modifyLiquidityRouter.modifyLiquidity(key, LIQ_PARAMS, ZERO_BYTES); - uint128 takeAmount = uint128(key.currency1.balanceOf(address(manager))); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - // Hook only takes currency 1 rn. - modifyLiquidityRouter.modifyLiquidity( - key, REMOVE_LIQ_PARAMS, abi.encode(takeAmount, AccessLockHook.LockAction.Take) - ); - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - // The balance of our contract should be equal to our original balance because we added and removed our liquidity. - // Note: the balance is off by one and is a known issue documented here: https://github.com/Uniswap/v3-core/issues/570 - assertTrue(balanceOfBefore0 + uint256(uint128(delta.amount0())) - balanceOfAfter0 <= 1); - assertTrue(balanceOfBefore1 + uint256(uint128(delta.amount1())) - balanceOfAfter1 - takeAmount <= 1); - assertEq(MockERC20(Currency.unwrap(currency1)).balanceOf(address(accessLockHook)), takeAmount); - } - - function test_beforeAddLiquidity_swap_succeedsWithAccessLock() public { - // Add liquidity so there is something to swap over. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - // Essentially "no-op"s the modifyPosition call and executes a swap before hand, applying the deltas from the swap to the locker. - modifyLiquidityRouter.modifyLiquidity( - key, IPoolManager.ModifyLiquidityParams(-120, 120, 1e18), abi.encode(amount, AccessLockHook.LockAction.Swap) - ); - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - // Balance decreases because we are swapping currency0 for currency1. - assertLt(balanceOfAfter0, balanceOfBefore0); - // Balance should be greater in currency1. - assertGt(balanceOfAfter1, balanceOfBefore1); - } - - function test_beforeAddLiquidity_addLiquidity_succeedsWithAccessLock() public { - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams(-120, 120, 10e18), - abi.encode(amount, AccessLockHook.LockAction.ModifyLiquidity) - ); - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - // Should have less balance in both currencies. - assertLt(balanceOfAfter0, balanceOfBefore0); - assertLt(balanceOfAfter1, balanceOfBefore1); - } - - function test_beforeAddLiquidity_donate_succeedsWithAccessLock() public { - // Add liquidity so there is a position to receive fees. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams(-120, 120, 1e18), - abi.encode(amount, AccessLockHook.LockAction.Donate) - ); - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - // Should have less balance in both currencies. - assertLt(balanceOfAfter0, balanceOfBefore0); - assertLt(balanceOfAfter1, balanceOfBefore1); - } - - function test_beforeAddLiquidity_burn_succeedsWithAccessLock() public { - // Add liquidity so there is a position to swap over. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - delta = swapRouter.swap( - key, - IPoolManager.SwapParams(true, 10000, TickMath.MIN_SQRT_RATIO + 1), - PoolSwapTest.TestSettings({withdrawTokens: false, settleUsingTransfer: true, currencyAlreadySent: false}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 amount1 = uint256(uint128(-delta.amount1())); - // We have some balance in the manager. - assertEq(manager.balanceOf(address(this), currency1.toId()), amount1); - manager.transfer(address(key.hooks), currency1.toId(), amount1); - assertEq(manager.balanceOf(address(key.hooks), currency1.toId()), amount1); - - delta = modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams(-120, 120, 1e18), - abi.encode(amount1, AccessLockHook.LockAction.Burn) - ); - - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - assertEq(balanceOfAfter1, balanceOfBefore1 + amount1 - uint256(uint128(delta.amount1()))); - } - - function test_beforeAddLiquidity_settle_succeedsWithAccessLock() public { - // Add liquidity so there is something to take. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - // Assertions in the hook. Takes and then settles within the hook. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams(-120, 120, 1e18), - abi.encode(amount, AccessLockHook.LockAction.Settle) - ); - } - - function test_beforeAddLiquidity_initialize_succeedsWithAccessLock() public { - // The hook intitializes a new pool with the new key at Constants.SQRT_RATIO_1_2; - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams(-120, 120, 1e18), - abi.encode(0, AccessLockHook.LockAction.Initialize) - ); - - PoolKey memory newKey = PoolKey({ - currency0: key.currency0, - currency1: key.currency1, - fee: Constants.FEE_LOW, - tickSpacing: 60, - hooks: IHooks(address(0)) - }); - (Pool.Slot0 memory slot0,,,) = manager.pools(newKey.toId()); - - assertEq(slot0.sqrtPriceX96, Constants.SQRT_RATIO_1_2); - } - - /** - * - * BEFORE SWAP TESTS - * - */ - function test_beforeSwap_mint_succeedsWithAccessLock() public { - // Add liquidity so there is something to swap against. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - // Small amount to swap (like NoOp). This way we can expect balances to just be from the hook applied delta. - delta = swapRouter.swap( - key, - IPoolManager.SwapParams(true, 1, TickMath.MIN_SQRT_RATIO + 1), - PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true, currencyAlreadySent: false}), - abi.encode(amount, AccessLockHook.LockAction.Mint) - ); - - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - assertEq(balanceOfBefore0 - balanceOfAfter0, uint256(uint128(delta.amount0()))); - // The balance of our contract should be from the modifyPositionRouter (delta) AND the hook (amount). - assertEq(balanceOfBefore1 - balanceOfAfter1, uint256(amount + uint256(uint128(delta.amount1())))); - - assertEq(manager.balanceOf(address(accessLockHook), currency1.toId()), amount); - } - - function test_beforeSwap_take_succeedsWithAccessLock() public { - // Add liquidity so there is something to take. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - // Hook only takes currency 1 rn. - // Use small amount to NoOp. - delta = swapRouter.swap( - key, - IPoolManager.SwapParams(true, 1, TickMath.MIN_SQRT_RATIO + 1), - PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true, currencyAlreadySent: false}), - abi.encode(amount, AccessLockHook.LockAction.Take) - ); - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - assertEq(balanceOfBefore0 - balanceOfAfter0, uint256(uint128(delta.amount0()))); - // The balance of our contract should be from the modifyLiquidityRouter (delta) AND the hook (amount). - assertEq(balanceOfBefore1 - balanceOfAfter1, uint256(amount + uint256(uint128(delta.amount1())))); - assertEq(MockERC20(Currency.unwrap(currency1)).balanceOf(address(accessLockHook)), amount); - } - - function test_beforeSwap_swap_succeedsWithAccessLock() public { - // Add liquidity so there is something to swap over. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - swapRouter.swap( - key, - // Use small amounts so that the zeroForOne swap is larger - IPoolManager.SwapParams(false, 1, TickMath.MAX_SQRT_RATIO - 1), - PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true, currencyAlreadySent: false}), - abi.encode(amount, AccessLockHook.LockAction.Swap) - ); - - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - // The larger swap is zeroForOne - // Balance decreases because we are swapping currency0 for currency1. - assertLt(balanceOfAfter0, balanceOfBefore0); - // Balance should be greater in currency1. - assertGt(balanceOfAfter1, balanceOfBefore1); - } - - function test_beforeSwap_addLiquidity_succeedsWithAccessLock() public { - // Add liquidity so there is something to swap over. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - // Make the swap amount small (like a NoOp). - swapRouter.swap( - key, - IPoolManager.SwapParams(true, 1, TickMath.MIN_SQRT_RATIO + 1), - PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true, currencyAlreadySent: false}), - abi.encode(amount, AccessLockHook.LockAction.ModifyLiquidity) - ); - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - // Should have less balance in both currencies. - assertLt(balanceOfAfter0, balanceOfBefore0); - assertLt(balanceOfAfter1, balanceOfBefore1); - } - - function test_beforeSwap_donate_succeedsWithAccessLock() public { - // Add liquidity so there is a position to receive fees. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - // Make the swap amount small (like a NoOp). - swapRouter.swap( - key, - IPoolManager.SwapParams(true, 1, TickMath.MIN_SQRT_RATIO + 1), - PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true, currencyAlreadySent: false}), - abi.encode(amount, AccessLockHook.LockAction.Donate) - ); - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - // Should have less balance in both currencies. - assertLt(balanceOfAfter0, balanceOfBefore0); - assertLt(balanceOfAfter1, balanceOfBefore1); - } - - /** - * - * BEFORE DONATE TESTS - * - */ - function test_beforeDonate_mint_succeedsWithAccessLock() public { - // Add liquidity so there is something to donate to. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - delta = donateRouter.donate(key, 1e18, 1e18, abi.encode(amount, AccessLockHook.LockAction.Mint)); - - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - assertEq(balanceOfBefore0 - balanceOfAfter0, uint256(uint128(delta.amount0()))); - // The balance of our contract should be from the donateRouter (delta) AND the hook (amount). - assertEq(balanceOfBefore1 - balanceOfAfter1, uint256(amount + uint256(uint128(delta.amount1())))); - - assertEq(manager.balanceOf(address(accessLockHook), currency1.toId()), amount); - } - - function test_beforeDonate_take_succeedsWithAccessLock() public { - // Add liquidity so there is something to take. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - // Hook only takes currency 1 rn. - delta = donateRouter.donate(key, 1e18, 1e18, abi.encode(amount, AccessLockHook.LockAction.Take)); - // Take applies a positive delta in currency1. - // Donate applies a positive delta in currency0 and currency1. - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - assertEq(balanceOfBefore0 - balanceOfAfter0, uint256(uint128(delta.amount0()))); - // The balance of our contract should be from the modifyLiquidityRouter (delta) AND the hook (amount). - assertEq(balanceOfBefore1 - balanceOfAfter1, uint256(amount + uint256(uint128(delta.amount1())))); - assertEq(MockERC20(Currency.unwrap(currency1)).balanceOf(address(accessLockHook)), amount); - } - - function test_beforeDonate_swap_succeedsWithAccessLock() public { - // Add liquidity so there is something to swap over. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - // Donate small amounts (NoOp) so we know the swap amount dominates. - donateRouter.donate(key, 1, 1, abi.encode(amount, AccessLockHook.LockAction.Swap)); - - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - // Balance of currency0 decreases bc we 1) donate and 2) swap zeroForOne. - assertLt(balanceOfAfter0, balanceOfBefore0); - // Since the donate amount is small, and we swapped zeroForOne, we expect balance of currency1 to increase. - assertGt(balanceOfAfter1, balanceOfBefore1); - } - - function test_beforeDonate_addLiquidity_succeedsWithAccessLock() public { - // Add liquidity so there is something to donate to. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - donateRouter.donate(key, 1e18, 1e18, abi.encode(amount, AccessLockHook.LockAction.ModifyLiquidity)); - - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - // Should have less balance in both currencies from adding liquidity AND donating. - assertLt(balanceOfAfter0, balanceOfBefore0); - assertLt(balanceOfAfter1, balanceOfBefore1); - } - - function test_beforeDonate_donate_succeedsWithAccessLock() public { - // Add liquidity so there is a position to receive fees. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - // Make the swap amount small (like a NoOp). - donateRouter.donate(key, 1e18, 1e18, abi.encode(amount, AccessLockHook.LockAction.Donate)); - - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - // Should have less balance in both currencies. - assertLt(balanceOfAfter0, balanceOfBefore0); - assertLt(balanceOfAfter1, balanceOfBefore1); - } - /** - * - * BEFORE INITIALIZE TESTS - * - */ - - function test_beforeInitialize_mint_succeedsWithAccessLock() public { - PoolKey memory key1 = PoolKey({ - currency0: currency0, - currency1: currency1, - fee: Constants.FEE_MEDIUM, - tickSpacing: 60, - hooks: IHooks(address(accessLockNoOpHook)) - }); - - initializeRouter.initialize(key1, SQRT_RATIO_1_1, abi.encode(amount, AccessLockHook.LockAction.Mint)); - - assertEq(manager.balanceOf(address(accessLockNoOpHook), currency1.toId()), amount); - } - - function test_beforeInitialize_take_succeedsWithAccessLock() public { - PoolKey memory key1 = PoolKey({ - currency0: currency0, - currency1: currency1, - fee: Constants.FEE_MEDIUM, - tickSpacing: 60, - hooks: IHooks(address(accessLockNoOpHook)) - }); - - // Add liquidity to a different pool there is something to take. - modifyLiquidityRouter.modifyLiquidity( - key, - IPoolManager.ModifyLiquidityParams({tickLower: -120, tickUpper: 120, liquidityDelta: 1000e18}), - ZERO_BYTES - ); - - initializeRouter.initialize(key1, SQRT_RATIO_1_1, abi.encode(amount, AccessLockHook.LockAction.Take)); - - assertEq(MockERC20(Currency.unwrap(currency1)).balanceOf(address(accessLockNoOpHook)), amount); - } - - function test_beforeInitialize_swap_revertsOnPoolNotInitialized() public { - PoolKey memory key1 = PoolKey({ - currency0: currency0, - currency1: currency1, - fee: Constants.FEE_MEDIUM, - tickSpacing: 60, - hooks: IHooks(address(accessLockNoOpHook)) - }); - - vm.expectRevert(IPoolManager.PoolNotInitialized.selector); - initializeRouter.initialize(key1, SQRT_RATIO_1_1, abi.encode(amount, AccessLockHook.LockAction.Swap)); - } - - function test_beforeInitialize_addLiquidity_revertsOnPoolNotInitialized() public { - PoolKey memory key1 = PoolKey({ - currency0: currency0, - currency1: currency1, - fee: Constants.FEE_MEDIUM, - tickSpacing: 60, - hooks: IHooks(address(accessLockNoOpHook)) - }); - - vm.expectRevert(IPoolManager.PoolNotInitialized.selector); - initializeRouter.initialize(key1, SQRT_RATIO_1_1, abi.encode(amount, AccessLockHook.LockAction.ModifyLiquidity)); - } - - function test_beforeInitialize_donate_revertsOnPoolNotInitialized() public { - PoolKey memory key1 = PoolKey({ - currency0: currency0, - currency1: currency1, - fee: Constants.FEE_MEDIUM, - tickSpacing: 60, - hooks: IHooks(address(accessLockNoOpHook)) - }); - - vm.expectRevert(IPoolManager.PoolNotInitialized.selector); - initializeRouter.initialize(key1, SQRT_RATIO_1_1, abi.encode(amount, AccessLockHook.LockAction.Donate)); - } - - /** - * - * HOOK FEE TESTS - * - */ - function test_hookFees_takesFeeOnWithdrawal() public { - (key,) = initPool( - currency0, currency1, IHooks(address(accessLockFeeHook)), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES - ); - - (uint256 userBalanceBefore0, uint256 poolBalanceBefore0, uint256 reservesBefore0) = _fetchBalances(currency0); - (uint256 userBalanceBefore1, uint256 poolBalanceBefore1, uint256 reservesBefore1) = _fetchBalances(currency1); - - // add liquidity - delta = modifyLiquidityRouter.modifyLiquidity(key, LIQ_PARAMS, ZERO_BYTES); - - (uint256 userBalanceAfter0, uint256 poolBalanceAfter0, uint256 reservesAfter0) = _fetchBalances(currency0); - (uint256 userBalanceAfter1, uint256 poolBalanceAfter1, uint256 reservesAfter1) = _fetchBalances(currency1); - - assert(delta.amount0() > 0 && delta.amount1() > 0); - assertEq(userBalanceBefore0 - uint128(delta.amount0()), userBalanceAfter0, "addLiq user balance currency0"); - assertEq(userBalanceBefore1 - uint128(delta.amount1()), userBalanceAfter1, "addLiq user balance currency1"); - assertEq(poolBalanceBefore0 + uint128(delta.amount0()), poolBalanceAfter0, "addLiq pool balance currency0"); - assertEq(poolBalanceBefore1 + uint128(delta.amount1()), poolBalanceAfter1, "addLiq pool balance currency1"); - assertEq(reservesBefore0 + uint128(delta.amount0()), reservesAfter0, "addLiq reserves currency0"); - assertEq(reservesBefore1 + uint128(delta.amount1()), reservesAfter1, "addLiq reserves currency1"); - - (userBalanceBefore0, poolBalanceBefore0, reservesBefore0) = - (userBalanceAfter0, poolBalanceAfter0, reservesAfter0); - (userBalanceBefore1, poolBalanceBefore1, reservesBefore1) = - (userBalanceAfter1, poolBalanceAfter1, reservesAfter1); - - // remove liquidity, a 40 bip fee should be taken - LIQ_PARAMS.liquidityDelta *= -1; - delta = modifyLiquidityRouter.modifyLiquidity(key, LIQ_PARAMS, ZERO_BYTES); - - (userBalanceAfter0, poolBalanceAfter0, reservesAfter0) = _fetchBalances(currency0); - (userBalanceAfter1, poolBalanceAfter1, reservesAfter1) = _fetchBalances(currency1); - - assert(delta.amount0() < 0 && delta.amount1() < 0); - - uint256 totalWithdraw0 = uint128(-delta.amount0()) - (uint128(-delta.amount0()) * 40 / 10000); - uint256 totalWithdraw1 = uint128(-delta.amount1()) - (uint128(-delta.amount1()) * 40 / 10000); - - assertEq(userBalanceBefore0 + totalWithdraw0, userBalanceAfter0, "removeLiq user balance currency0"); - assertEq(userBalanceBefore1 + totalWithdraw1, userBalanceAfter1, "removeLiq user balance currency1"); - assertEq(poolBalanceBefore0 - uint128(-delta.amount0()), poolBalanceAfter0, "removeLiq pool balance currency0"); - assertEq(poolBalanceBefore1 - uint128(-delta.amount1()), poolBalanceAfter1, "removeLiq pool balance currency1"); - assertEq(reservesBefore0 - uint128(-delta.amount0()), reservesAfter0, "removeLiq reserves currency0"); - assertEq(reservesBefore1 - uint128(-delta.amount1()), reservesAfter1, "removeLiq reserves currency1"); - } - - function test_hookFees_takesFeeOnInputOfSwap() public { - (key,) = initPool( - currency0, currency1, IHooks(address(accessLockFeeHook)), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES - ); - - // add liquidity - delta = modifyLiquidityRouter.modifyLiquidity(key, LIQ_PARAMS, ZERO_BYTES); - - // now swap, with a hook fee of 55 bips - (uint256 userBalanceBefore0, uint256 poolBalanceBefore0, uint256 reservesBefore0) = _fetchBalances(currency0); - (uint256 userBalanceBefore1, uint256 poolBalanceBefore1, uint256 reservesBefore1) = _fetchBalances(currency1); - - delta = swapRouter.swap( - key, - IPoolManager.SwapParams({zeroForOne: true, amountSpecified: 100000, sqrtPriceLimitX96: SQRT_RATIO_1_2}), - PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true, currencyAlreadySent: false}), - ZERO_BYTES - ); - - assert(delta.amount0() > 0 && delta.amount1() < 0); - - uint256 amountIn0 = uint128(delta.amount0()); - uint256 userAmountOut1 = uint128(-delta.amount1()) - (uint128(-delta.amount1()) * 55 / 10000); - - (uint256 userBalanceAfter0, uint256 poolBalanceAfter0, uint256 reservesAfter0) = _fetchBalances(currency0); - (uint256 userBalanceAfter1, uint256 poolBalanceAfter1, uint256 reservesAfter1) = _fetchBalances(currency1); - - assertEq(userBalanceBefore0 - amountIn0, userBalanceAfter0, "swap user balance currency0"); - assertEq(userBalanceBefore1 + userAmountOut1, userBalanceAfter1, "swap user balance currency1"); - assertEq(poolBalanceBefore0 + amountIn0, poolBalanceAfter0, "swap pool balance currency0"); - assertEq(poolBalanceBefore1 - uint128(-delta.amount1()), poolBalanceAfter1, "swap pool balance currency1"); - assertEq(reservesBefore0 + amountIn0, reservesAfter0, "swap reserves currency0"); - assertEq(reservesBefore1 - uint128(-delta.amount1()), reservesAfter1, "swap reserves currency1"); - } - - /** - * - * EDGE CASE TESTS - * - */ - function test_onlyByLocker_revertsWhenHookIsNotCurrentHook() public { - // Call first access lock hook. Should succeed. - uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - - delta = modifyLiquidityRouter.modifyLiquidity( - key, IPoolManager.ModifyLiquidityParams(0, 60, 1e18), abi.encode(amount, AccessLockHook.LockAction.Mint) - ); - - uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); - - assertEq(balanceOfBefore0 - balanceOfAfter0, uint256(uint128(delta.amount0()))); - // The balance of our contract should be from the modifyLiquidityRouter (delta) AND the hook (amount). - assertEq(balanceOfBefore1 - balanceOfAfter1, uint256(amount + uint256(uint128(delta.amount1())))); - - assertEq(manager.balanceOf(address(accessLockHook), currency1.toId()), amount); - - assertEq(address(manager.getCurrentHook()), address(0)); - - (PoolKey memory keyAccessLockHook2,) = - initPool(currency0, currency1, IHooks(accessLockHook2), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES); - - // Delegates the beforeAddLiquidity call to the hook in `key` which tries to mint on manager - // but reverts because hook in `key` is not the current hook. - vm.expectRevert( - abi.encodeWithSelector( - IPoolManager.LockedBy.selector, address(modifyLiquidityRouter), address(accessLockHook2) - ) - ); - delta = modifyLiquidityRouter.modifyLiquidity( - keyAccessLockHook2, IPoolManager.ModifyLiquidityParams(0, 60, 1e18), abi.encode(true, key) - ); - } - - function test_onlyByLocker_succeedsAfterHookMakesNestedCall() public { - (PoolKey memory keyWithNoHook,) = - initPool(currency0, currency1, IHooks(address(0)), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES); - - (PoolKey memory keyAccessLockHook2,) = - initPool(currency0, currency1, IHooks(accessLockHook2), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES); - - modifyLiquidityRouter.modifyLiquidity( - keyAccessLockHook2, IPoolManager.ModifyLiquidityParams(0, 60, 1e18), abi.encode(false, keyWithNoHook) - ); - assertEq(manager.balanceOf(address(accessLockHook2), currency1.toId()), 10); - } - - function test_onlyByLocker_revertsWhenThereIsNoOutsideLock() public { - modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, 1e18), ZERO_BYTES); - assertEq(address(manager.getCurrentHook()), address(0)); - - vm.expectRevert(abi.encodeWithSelector(IPoolManager.LockedBy.selector, address(0), address(0))); - vm.prank(address(key.hooks)); - manager.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(0, 60, 1e18), ZERO_BYTES); - } - - function test_getCurrentHook_isClearedAfterNestedLock() public { - // Create pool for AccessLockHook3. - (PoolKey memory keyAccessLockHook3,) = - initPool(currency0, currency1, IHooks(accessLockHook3), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES); - // Fund AccessLockHook3 with currency0. - MockERC20(Currency.unwrap(currency0)).transfer(address(accessLockHook3), 10); - assertEq(MockERC20(Currency.unwrap(currency0)).balanceOf(address(accessLockHook3)), 10); - - // Create pool to donate 10 of currency0 to inside of AccessLockHook3. This means AccessLockHook3 must acquire a new lock and settle. - // The currentHook addresses are checked inside this nested lock. - (PoolKey memory _key,) = - initPool(currency0, currency1, IHooks(address(0)), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES); - // Add liquidity so that the AccessLockHook3 can donate to something. - modifyLiquidityRouter.modifyLiquidity( - _key, IPoolManager.ModifyLiquidityParams(-60, 60, 10 * 10 ** 18), ZERO_BYTES - ); - accessLockHook3.setKey(_key); - - // Asserts are in the AccessLockHook3. - modifyLiquidityRouter.modifyLiquidity( - keyAccessLockHook3, IPoolManager.ModifyLiquidityParams(0, 60, 1e18), ZERO_BYTES - ); - } - - function test_getCurrentHook_isClearedAfterNoOpOnAllHooks() public { - (PoolKey memory noOpKey,) = - initPool(currency0, currency1, IHooks(accessLockNoOpHook), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES); - - // Assertions for current hook address in AccessLockHook and respective routers. - // beforeAddLiquidity noOp - modifyLiquidityRouter.modifyLiquidity( - noOpKey, - IPoolManager.ModifyLiquidityParams({tickLower: 0, tickUpper: 60, liquidityDelta: 1e18}), - abi.encode(0, AccessLockHook.LockAction.NoOp) - ); - - // beforeDonate noOp - donateRouter.donate(noOpKey, 1e18, 1e18, abi.encode(0, AccessLockHook.LockAction.NoOp)); - - // beforeSwap noOp - swapRouter.swap( - noOpKey, - IPoolManager.SwapParams(true, 1, TickMath.MIN_SQRT_RATIO + 1), - PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true, currencyAlreadySent: false}), - abi.encode(0, AccessLockHook.LockAction.NoOp) - ); - } - - function _fetchBalances(Currency currency) - internal - view - returns (uint256 userBalance, uint256 poolBalance, uint256 reserves) - { - userBalance = currency.balanceOf(address(this)); - poolBalance = currency.balanceOf(address(manager)); - reserves = manager.reservesOf(currency); - } -} diff --git a/test/CurrentHookAddress.t.sol b/test/CurrentHookAddress.t.sol deleted file mode 100644 index a23c49ec4..000000000 --- a/test/CurrentHookAddress.t.sol +++ /dev/null @@ -1,46 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.24; - -import {Test} from "forge-std/Test.sol"; -import {Lockers} from "../src/libraries/Lockers.sol"; -import {IHooks} from "../src/interfaces/IHooks.sol"; - -contract CurrentHookAddressTest is Test { - function test_getCurrentHook() public { - assertEq(address(Lockers.getCurrentHook()), address(0)); - } - - function test_setCurrentHook() public { - Lockers.setCurrentHook(IHooks(address(1))); - assertEq(address(Lockers.getCurrentHook()), address(1)); - } - - function test_setCurrentHook_TwiceDoesNotSucceed() public { - (bool set) = Lockers.setCurrentHook(IHooks(address(1))); - assertTrue(set); - set = Lockers.setCurrentHook(IHooks(address(2))); - assertFalse(set); - assertEq(address(Lockers.getCurrentHook()), address(1)); - } - - function test_clearCurrentHook() public { - Lockers.setCurrentHook(IHooks(address(1))); - assertEq(address(Lockers.getCurrentHook()), address(1)); - Lockers.clearCurrentHook(); - assertEq(address(Lockers.getCurrentHook()), address(0)); - } - - function test_setCurrentHook_afterLock() public { - Lockers.push(address(this), address(this)); - Lockers.setCurrentHook(IHooks(address(1))); - assertEq(address(Lockers.getCurrentHook()), address(1)); - } - - function test_setCurrentHook_beforeAndAfterLock() public { - Lockers.push(address(this), address(this)); - Lockers.setCurrentHook(IHooks(address(2))); - assertEq(address(Lockers.getCurrentHook()), address(2)); - Lockers.push(address(1), address(1)); - assertEq(address(Lockers.getCurrentHook()), address(0)); - } -} diff --git a/test/Hooks.t.sol b/test/Hooks.t.sol index 645fe1bfa..b04c64d8d 100644 --- a/test/Hooks.t.sol +++ b/test/Hooks.t.sol @@ -18,7 +18,6 @@ import {Deployers} from "./utils/Deployers.sol"; import {Fees} from "../src/Fees.sol"; import {PoolId, PoolIdLibrary} from "../src/types/PoolId.sol"; import {PoolKey} from "../src/types/PoolKey.sol"; -import {AccessLockHook} from "../src/test/AccessLockHook.sol"; import {IERC20Minimal} from "../src/interfaces/external/IERC20Minimal.sol"; import {BalanceDelta} from "../src/types/BalanceDelta.sol"; @@ -211,8 +210,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -226,7 +224,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_beforeInitialize(uint160 addr) public { @@ -246,8 +243,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertTrue(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -261,7 +257,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_afterInitialize(uint160 addr) public { @@ -281,8 +276,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -296,7 +290,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_beforeAndAfterInitialize(uint160 addr) public { @@ -315,8 +308,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertTrue(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -330,7 +322,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_beforeAddLiquidity(uint160 addr) public { @@ -349,8 +340,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -364,7 +354,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_afterAddLiquidity(uint160 addr) public { @@ -383,8 +372,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -398,7 +386,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_beforeAndAfterAddLiquidity(uint160 addr) public { @@ -418,8 +405,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -433,7 +419,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_beforeRemoveLiquidity(uint160 addr) public { @@ -452,8 +437,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -467,7 +451,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_afterRemoveLiquidity(uint160 addr) public { @@ -486,8 +469,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -501,7 +483,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_beforeAfterRemoveLiquidity(uint160 addr) public { @@ -521,8 +502,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -536,7 +516,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_beforeInitializeAfterAddLiquidity(uint160 addr) public { @@ -556,8 +535,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertTrue(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -571,7 +549,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_beforeSwap(uint160 addr) public { @@ -590,8 +567,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -605,7 +581,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_afterSwap(uint160 addr) public { @@ -624,8 +599,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: true, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -639,7 +613,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_beforeAndAfterSwap(uint160 addr) public { @@ -658,8 +631,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: true, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -673,7 +645,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_beforeDonate(uint160 addr) public { @@ -692,8 +663,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: true, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -707,7 +677,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertTrue(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_afterDonate(uint160 addr) public { @@ -726,8 +695,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: true, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -741,7 +709,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertTrue(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_beforeAndAfterDonate(uint160 addr) public { @@ -760,8 +727,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: true, afterDonate: true, - noOp: false, - accessLock: false + noOp: false }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -775,41 +741,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertTrue(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertTrue(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); - } - - function test_validateHookAddress_accessLock(uint160 addr) public { - uint160 preAddr = uint160(uint256(addr) & clearAllHookPermisssionsMask); - IHooks hookAddr = IHooks(address(uint160(preAddr | Hooks.ACCESS_LOCK_FLAG))); - Hooks.validateHookPermissions( - hookAddr, - Hooks.Permissions({ - beforeInitialize: false, - afterInitialize: false, - beforeAddLiquidity: false, - afterAddLiquidity: false, - beforeRemoveLiquidity: false, - afterRemoveLiquidity: false, - beforeSwap: false, - afterSwap: false, - beforeDonate: false, - afterDonate: false, - noOp: false, - accessLock: true - }) - ); - assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.AFTER_INITIALIZE_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.BEFORE_ADD_LIQUIDITY_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.AFTER_ADD_LIQUIDITY_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.BEFORE_REMOVE_LIQUIDITY_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.AFTER_REMOVE_LIQUIDITY_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.BEFORE_SWAP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.AFTER_SWAP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertTrue(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_allHooks(uint160 addr) public { @@ -829,8 +760,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: true, beforeDonate: true, afterDonate: true, - noOp: true, - accessLock: true + noOp: true }) ); assertTrue(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -844,7 +774,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertTrue(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertTrue(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertTrue(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertTrue(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_noOp(uint160 addr) public { @@ -870,8 +799,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: true, afterDonate: false, - noOp: true, - accessLock: false + noOp: true }) ); assertFalse(hookAddr.hasPermission(Hooks.BEFORE_INITIALIZE_FLAG)); @@ -885,7 +813,6 @@ contract HooksTest is Test, Deployers, GasSnapshot { assertTrue(hookAddr.hasPermission(Hooks.BEFORE_DONATE_FLAG)); assertFalse(hookAddr.hasPermission(Hooks.AFTER_DONATE_FLAG)); assertTrue(hookAddr.hasPermission(Hooks.NO_OP_FLAG)); - assertFalse(hookAddr.hasPermission(Hooks.ACCESS_LOCK_FLAG)); } function test_validateHookAddress_failsAllHooks(uint152 addr, uint8 mask) public { @@ -906,8 +833,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: true, beforeDonate: true, afterDonate: true, - noOp: true, - accessLock: true + noOp: true }) ); } @@ -931,8 +857,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { afterSwap: false, beforeDonate: false, afterDonate: false, - noOp: false, - accessLock: false + noOp: false }) ); } diff --git a/test/Locker.t.sol b/test/Locker.t.sol new file mode 100644 index 000000000..2093c8d8f --- /dev/null +++ b/test/Locker.t.sol @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; +import {Locker} from "../src/libraries/Locker.sol"; + +contract LockerTest is Test { + address constant ADDRESS_AS = 0xaAaAaAaaAaAaAaaAaAAAAAAAAaaaAaAaAaaAaaAa; + address constant ADDRESS_BS = 0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB; + + function test_fuzz_setLocker(address locker) public { + assertEq(address(Locker.getLocker()), address(0)); + + if (locker == address(0)) { + vm.expectRevert(Locker.InvalidLocker.selector); + Locker.setLocker(locker); + } else { + Locker.setLocker(locker); + assertEq(address(Locker.getLocker()), locker); + } + } + + function test_fuzz_clearLocker(address locker) public { + vm.assume(locker != address(0)); + Locker.setLocker(locker); + + assertEq(address(Locker.getLocker()), locker); + + Locker.clearLocker(); + + assertEq(address(Locker.getLocker()), address(0)); + } + + function test_fuzz_isLocked(address locker) public { + vm.assume(locker != address(0)); + assertFalse(Locker.isLocked()); + + Locker.setLocker(locker); + + assertTrue(Locker.isLocked()); + + Locker.clearLocker(); + + assertFalse(Locker.isLocked()); + } +} diff --git a/test/LockersLibrary.t.sol b/test/LockersLibrary.t.sol deleted file mode 100644 index 587fad603..000000000 --- a/test/LockersLibrary.t.sol +++ /dev/null @@ -1,148 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.24; - -import {Test} from "forge-std/Test.sol"; -import {Vm} from "forge-std/Vm.sol"; -import {PoolManager} from "../src/PoolManager.sol"; -import {IPoolManager} from "../src/interfaces/IPoolManager.sol"; -import {Deployers} from "./utils/Deployers.sol"; -import {ILockCallback} from "../src/interfaces/callback/ILockCallback.sol"; -import {Currency, CurrencyLibrary} from "../src/types/Currency.sol"; -import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; -import {PoolKey} from "../src/types/PoolKey.sol"; -import {IHooks} from "../src/interfaces/IHooks.sol"; -import {Lockers} from "../src/libraries/Lockers.sol"; - -contract LockersLibraryTest is Test, Deployers, ILockCallback { - using CurrencyLibrary for Currency; - - uint256 constant LOCKERS_OFFSET = uint256(1); - - function setUp() public { - initializeManagerRoutersAndPoolsWithLiq(IHooks(address(0))); - } - - function testLockerLengthAndNonzeroDeltaCount() public { - (uint256 lengthDuringLockCallback, uint256 nonzeroDeltaCountDuringCallback) = - abi.decode(manager.lock(address(this), ""), (uint256, uint256)); - assertEq(lengthDuringLockCallback, 1); - assertEq(nonzeroDeltaCountDuringCallback, 1); - assertEq(manager.getLockLength(), 0); - assertEq(manager.getLockNonzeroDeltaCount(), 0); - } - - function lockAcquired(address, bytes calldata) public returns (bytes memory) { - uint256 len = manager.getLockLength(); - - // apply a delta and save count - manager.take(key.currency0, address(this), 1); - uint256 count = manager.getLockNonzeroDeltaCount(); - key.currency0.transfer(address(manager), 1); - manager.settle(key.currency0); - - bytes memory data = abi.encode(len, count); - return data; - } - - function test_push() public { - Lockers.push(address(this), address(1)); - assertEq(Lockers.length(), 1); - assertEq(Lockers.getLocker(LOCKERS_OFFSET), address(this)); - assertEq(Lockers.getLockCaller(LOCKERS_OFFSET), address(1)); - } - - function test_push_multipleAddressesFuzz(address[2][] memory addrs) public { - for (uint256 i = 0; i < addrs.length; i++) { - address[2] memory loopAddrs = addrs[i]; - address locker = loopAddrs[0]; - address lockCaller = loopAddrs[1]; - assertEq(Lockers.length(), i); - Lockers.push(locker, lockCaller); - assertEq(Lockers.length(), LOCKERS_OFFSET + i); - assertEq(Lockers.getLocker(LOCKERS_OFFSET + i), locker); - assertEq(Lockers.getLockCaller(LOCKERS_OFFSET + i), lockCaller); - } - } - - function test_getCurrentLocker_multipleAddressesFuzz(address[2][] memory addrs) public { - for (uint256 i = 0; i < addrs.length; i++) { - address[2] memory loopAddrs = addrs[i]; - address locker = loopAddrs[0]; - address lockCaller = loopAddrs[1]; - assertEq(Lockers.length(), i); - Lockers.push(locker, lockCaller); - assertEq(Lockers.length(), LOCKERS_OFFSET + i); - assertEq(Lockers.getCurrentLocker(), locker); - assertEq(Lockers.getCurrentLockCaller(), lockCaller); - } - } - - function test_pop() public { - Lockers.push(address(this), address(1)); - assertEq(Lockers.length(), 1); - Lockers.pop(); - assertEq(Lockers.length(), 0); - } - - function test_pop_multipleAddressesFuzz(address[2][] memory addrs) public { - for (uint256 i = 0; i < addrs.length; i++) { - address[2] memory loopAddrs = addrs[i]; - address locker = loopAddrs[0]; - address lockCaller = loopAddrs[1]; - Lockers.push(locker, lockCaller); - } - - assertEq(Lockers.length(), addrs.length); - - for (uint256 i = 0; i < addrs.length; i++) { - Lockers.pop(); - assertEq(Lockers.length(), addrs.length - i - LOCKERS_OFFSET); - } - - assertEq(Lockers.length(), 0); - } - - function test_clear(address[2][] memory addrs) public { - for (uint256 i = 0; i < addrs.length; i++) { - address[2] memory loopAddrs = addrs[i]; - address locker = loopAddrs[0]; - address lockCaller = loopAddrs[1]; - Lockers.push(locker, lockCaller); - } - - assertEq(Lockers.length(), addrs.length); - Lockers.clear(); - assertEq(Lockers.length(), 0); - } - - function test_incrementNonzeroDeltaCount() public { - Lockers.incrementNonzeroDeltaCount(); - assertEq(Lockers.nonzeroDeltaCount(), 1); - } - - function test_incrementNonzeroDeltaCountFuzz(uint8 count) public { - for (uint256 i = 0; i < count; i++) { - Lockers.incrementNonzeroDeltaCount(); - assertEq(Lockers.nonzeroDeltaCount(), i + 1); - } - } - - function test_decrementNonzeroDeltaCount() public { - Lockers.incrementNonzeroDeltaCount(); - Lockers.decrementNonzeroDeltaCount(); - assertEq(Lockers.nonzeroDeltaCount(), 0); - } - - function test_decrementNonzeroDeltaCountFuzz(uint8 count) public { - for (uint256 i = 0; i < count; i++) { - Lockers.incrementNonzeroDeltaCount(); - } - - assertEq(Lockers.nonzeroDeltaCount(), count); - - for (uint256 i = 0; i < count; i++) { - Lockers.decrementNonzeroDeltaCount(); - assertEq(Lockers.nonzeroDeltaCount(), count - i - 1); - } - } -} diff --git a/test/NestedActions.t.sol b/test/NestedActions.t.sol new file mode 100644 index 000000000..e2e8cd5fc --- /dev/null +++ b/test/NestedActions.t.sol @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; +import {Deployers} from "./utils/Deployers.sol"; +import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol"; +import {Action} from "../src/test/PoolNestedActionsTest.sol"; +import {IHooks} from "../src/interfaces/IHooks.sol"; + +contract NestedActions is Test, Deployers, GasSnapshot { + Action[] actions; + + function setUp() public { + initializeManagerRoutersAndPoolsWithLiq(IHooks(address(0))); + } + + // Asserts and expected outcomes are tested inside the nestedActionRouter + + function test_nestedSwap() public { + actions = [Action.SWAP_AND_SETTLE]; + manager.lock(address(nestedActionRouter), abi.encode(actions)); + } + + function test_nestedAddLiquidity() public { + actions = [Action.ADD_LIQ_AND_SETTLE]; + manager.lock(address(nestedActionRouter), abi.encode(actions)); + } + + function test_nestedRemoveLiquidity() public { + actions = [Action.ADD_LIQ_AND_SETTLE, Action.REMOVE_LIQ_AND_SETTLE]; + manager.lock(address(nestedActionRouter), abi.encode(actions)); + } + + function test_nestedDonate() public { + actions = [Action.DONATE_AND_SETTLE]; + manager.lock(address(nestedActionRouter), abi.encode(actions)); + } +} diff --git a/test/NonZeroDeltaCount.t.sol b/test/NonZeroDeltaCount.t.sol new file mode 100644 index 000000000..9f37f5ce0 --- /dev/null +++ b/test/NonZeroDeltaCount.t.sol @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; +import {NonZeroDeltaCount} from "../src/libraries/NonZeroDeltaCount.sol"; + +contract NonZeroDeltaCountTest is Test { + address constant ADDRESS_AS = 0xaAaAaAaaAaAaAaaAaAAAAAAAAaaaAaAaAaaAaaAa; + address constant ADDRESS_BS = 0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB; + + function test_incrementNonzeroDeltaCount() public { + NonZeroDeltaCount.increment(); + assertEq(NonZeroDeltaCount.read(), 1); + } + + function test_decrementNonzeroDeltaCount() public { + NonZeroDeltaCount.increment(); + NonZeroDeltaCount.decrement(); + assertEq(NonZeroDeltaCount.read(), 0); + } + + // Reading from right to left. Bit of 0: call increase. Bit of 1: call decrease. + // The library allows over over/underflow so we dont check for that here + function test_fuzz_nonZeroDeltaCount(uint256 instructions) public { + uint256 expectedCount; + for (uint256 i = 0; i < 256; i++) { + if ((instructions & (1 << i)) == 0) { + NonZeroDeltaCount.increment(); + unchecked { + expectedCount++; + } + } else { + NonZeroDeltaCount.decrement(); + unchecked { + expectedCount--; + } + } + assertEq(NonZeroDeltaCount.read(), expectedCount); + } + } +} diff --git a/test/PoolManager.t.sol b/test/PoolManager.t.sol index 7e0782c45..cc7adf1e7 100644 --- a/test/PoolManager.t.sol +++ b/test/PoolManager.t.sol @@ -2,7 +2,6 @@ pragma solidity ^0.8.20; import {Test} from "forge-std/Test.sol"; -import {Vm} from "forge-std/Vm.sol"; import {IHooks} from "../src/interfaces/IHooks.sol"; import {Hooks} from "../src/libraries/Hooks.sol"; import {IPoolManager} from "../src/interfaces/IPoolManager.sol"; @@ -23,12 +22,13 @@ import {BalanceDelta, BalanceDeltaLibrary} from "../src/types/BalanceDelta.sol"; import {PoolSwapTest} from "../src/test/PoolSwapTest.sol"; import {TestInvalidERC20} from "../src/test/TestInvalidERC20.sol"; import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol"; -import {PoolLockTest} from "../src/test/PoolLockTest.sol"; +import {PoolEmptyLockTest} from "../src/test/PoolEmptyLockTest.sol"; +import {Action} from "../src/test/PoolNestedActionsTest.sol"; import {PoolId, PoolIdLibrary} from "../src/types/PoolId.sol"; import {FeeLibrary} from "../src/libraries/FeeLibrary.sol"; import {Position} from "../src/libraries/Position.sol"; +import {Constants} from "./utils/Constants.sol"; import {SafeCast} from "../src/libraries/SafeCast.sol"; -import {LiquidityAmounts} from "./utils/LiquidityAmounts.sol"; import {AmountHelpers} from "./utils/AmountHelpers.sol"; contract PoolManagerTest is Test, Deployers, GasSnapshot { @@ -57,16 +57,12 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { address caller, address indexed sender, address indexed receiver, uint256 indexed id, uint256 amount ); - PoolLockTest lockTest; - - address ADDRESS_ZERO = address(0); - address EMPTY_HOOKS = address(0xf000000000000000000000000000000000000000); - address MOCK_HOOKS = address(0xfF00000000000000000000000000000000000000); + PoolEmptyLockTest emptyLockRouter; function setUp() public { initializeManagerRoutersAndPoolsWithLiq(IHooks(address(0))); - lockTest = new PoolLockTest(manager); + emptyLockRouter = new PoolEmptyLockTest(manager); } function test_bytecodeSize() public { @@ -157,7 +153,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { address payable mockAddr = payable(address(uint160(Hooks.BEFORE_ADD_LIQUIDITY_FLAG | Hooks.AFTER_ADD_LIQUIDITY_FLAG))); - address payable hookAddr = payable(MOCK_HOOKS); + address payable hookAddr = payable(Constants.MOCK_HOOKS); vm.etch(hookAddr, vm.getDeployedCode("EmptyTestHooks.sol:EmptyTestHooks")); MockContract mockContract = new MockContract(); @@ -185,7 +181,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { address payable mockAddr = payable(address(uint160(Hooks.BEFORE_REMOVE_LIQUIDITY_FLAG | Hooks.AFTER_REMOVE_LIQUIDITY_FLAG))); - address payable hookAddr = payable(MOCK_HOOKS); + address payable hookAddr = payable(Constants.MOCK_HOOKS); vm.etch(hookAddr, vm.getDeployedCode("EmptyTestHooks.sol:EmptyTestHooks")); MockContract mockContract = new MockContract(); @@ -384,7 +380,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { } function test_addLiquidity_withHooks_gas() public { - address hookEmptyAddr = EMPTY_HOOKS; + address hookEmptyAddr = Constants.EMPTY_HOOKS; MockHooks impl = new MockHooks(); vm.etch(hookEmptyAddr, address(impl).code); MockHooks mockHooks = MockHooks(hookEmptyAddr); @@ -397,7 +393,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { } function test_removeLiquidity_withHooks_gas() public { - address hookEmptyAddr = EMPTY_HOOKS; + address hookEmptyAddr = Constants.EMPTY_HOOKS; MockHooks impl = new MockHooks(); vm.etch(hookEmptyAddr, address(impl).code); MockHooks mockHooks = MockHooks(hookEmptyAddr); @@ -411,7 +407,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { } function test_mint_withHooks_EOAInitiated() public { - address hookEmptyAddr = EMPTY_HOOKS; + address hookEmptyAddr = Constants.EMPTY_HOOKS; MockHooks impl = new MockHooks(); vm.etch(hookEmptyAddr, address(impl).code); MockHooks mockHooks = MockHooks(hookEmptyAddr); @@ -571,7 +567,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { function test_swap_succeedsWithHooksIfInitialized() public { address payable mockAddr = payable(address(uint160(Hooks.BEFORE_SWAP_FLAG | Hooks.AFTER_SWAP_FLAG))); - address payable hookAddr = payable(MOCK_HOOKS); + address payable hookAddr = payable(Constants.MOCK_HOOKS); vm.etch(hookAddr, vm.getDeployedCode("EmptyTestHooks.sol:EmptyTestHooks")); MockContract mockContract = new MockContract(); @@ -678,7 +674,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { } function test_swap_withHooks_gas() public { - address hookEmptyAddr = EMPTY_HOOKS; + address hookEmptyAddr = Constants.EMPTY_HOOKS; MockHooks impl = new MockHooks(); vm.etch(hookEmptyAddr, address(impl).code); @@ -1386,16 +1382,28 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { assertEq(manager.protocolFeesAccrued(nativeCurrency), 0); } - function test_lock_NoOpIsOk() public { + function test_lock_NoOp_gas() public { snapStart("gas overhead of no-op lock"); - lockTest.lock(); + emptyLockRouter.lock(); snapEnd(); } function test_lock_EmitsCorrectId() public { vm.expectEmit(false, false, false, true); emit LockAcquired(); - lockTest.lock(); + emptyLockRouter.lock(); + } + + Action[] actions; + + function test_lock_cannotBeCalledTwiceByLocker() public { + actions = [Action.NESTED_SELF_LOCK]; + manager.lock(address(nestedActionRouter), abi.encode(actions)); + } + + function test_lock_cannotBeCalledTwiceByDifferentLockers() public { + actions = [Action.NESTED_EXECUTOR_LOCK]; + manager.lock(address(nestedActionRouter), abi.encode(actions)); } // function testExtsloadForPoolPrice() public { diff --git a/test/PoolManagerInitialize.t.sol b/test/PoolManagerInitialize.t.sol index 313e439d7..75a8d8544 100644 --- a/test/PoolManagerInitialize.t.sol +++ b/test/PoolManagerInitialize.t.sol @@ -10,6 +10,7 @@ import {PoolManager} from "../src/PoolManager.sol"; import {TickMath} from "../src/libraries/TickMath.sol"; import {Pool} from "../src/libraries/Pool.sol"; import {Deployers} from "./utils/Deployers.sol"; +import {Constants} from "./utils/Constants.sol"; import {Currency, CurrencyLibrary} from "../src/types/Currency.sol"; import {MockHooks} from "../src/test/MockHooks.sol"; import {MockContract} from "../src/test/MockContract.sol"; @@ -35,11 +36,6 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { IHooks hooks ); - address ADDRESS_ZERO = address(0); - address EMPTY_HOOKS = address(0xf000000000000000000000000000000000000000); - address ALL_HOOKS = address(0xff00000000000000000000000000000000000001); - address MOCK_HOOKS = address(0xfF00000000000000000000000000000000000000); - function setUp() public { deployFreshManagerAndRouters(); (currency0, currency1) = deployMintAndApprove2Currencies(); @@ -48,7 +44,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { currency0: currency0, currency1: currency1, fee: 3000, - hooks: IHooks(ADDRESS_ZERO), + hooks: IHooks(Constants.ADDRESS_ZERO), tickSpacing: 60 }); } @@ -58,7 +54,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_RATIO, TickMath.MAX_SQRT_RATIO - 1)); // tested in Hooks.t.sol - key0.hooks = IHooks(ADDRESS_ZERO); + key0.hooks = IHooks(Constants.ADDRESS_ZERO); if (key0.fee & FeeLibrary.STATIC_FEE_MASK >= 1000000) { vm.expectRevert(abi.encodeWithSelector(IFees.FeeTooLarge.selector)); @@ -113,7 +109,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_RATIO, TickMath.MAX_SQRT_RATIO - 1)); address payable mockAddr = payable(address(uint160(Hooks.BEFORE_INITIALIZE_FLAG | Hooks.AFTER_INITIALIZE_FLAG))); - address payable hookAddr = payable(MOCK_HOOKS); + address payable hookAddr = payable(Constants.MOCK_HOOKS); vm.etch(hookAddr, vm.getDeployedCode("EmptyTestHooks.sol:EmptyTestHooks")); MockContract mockContract = new MockContract(); @@ -163,7 +159,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { // Assumptions tested in Pool.t.sol sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_RATIO, TickMath.MAX_SQRT_RATIO - 1)); - address hookEmptyAddr = EMPTY_HOOKS; + address hookEmptyAddr = Constants.EMPTY_HOOKS; MockHooks impl = new MockHooks(); vm.etch(hookEmptyAddr, address(impl).code); diff --git a/test/PoolManagerReentrancyTest.t.sol b/test/PoolManagerReentrancyTest.t.sol deleted file mode 100644 index 8eb25d4c6..000000000 --- a/test/PoolManagerReentrancyTest.t.sol +++ /dev/null @@ -1,171 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.20; - -import {Test} from "forge-std/Test.sol"; -import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; -import {Currency, CurrencyLibrary} from "../src/types/Currency.sol"; -import {IPoolManager} from "../src/interfaces/IPoolManager.sol"; -import {IHooks} from "../src/interfaces/IHooks.sol"; -import {ILockCallback} from "../src/interfaces/callback/ILockCallback.sol"; -import {PoolManager} from "../src/PoolManager.sol"; -import {Deployers} from "./utils/Deployers.sol"; - -contract TokenLocker is ILockCallback { - using CurrencyLibrary for Currency; - - function main(IPoolManager manager, Currency currency, bool reclaim) external { - manager.lock(address(this), abi.encode(currency, reclaim)); - } - - function lockAcquired(address, bytes calldata data) external returns (bytes memory) { - (Currency currency, bool reclaim) = abi.decode(data, (Currency, bool)); - - IPoolManager manager = IPoolManager(msg.sender); - uint256 count = manager.getLockNonzeroDeltaCount(); - assert(count == 0); - - int256 delta = manager.currencyDelta(address(this), currency); - assert(delta == 0); - - // deposit some tokens - currency.transfer(address(manager), 1); - manager.settle(currency); - count = manager.getLockNonzeroDeltaCount(); - assert(count == 1); - delta = manager.currencyDelta(address(this), currency); - assert(delta == -1); - - // take them back - if (reclaim) { - manager.take(currency, address(this), 1); - count = manager.getLockNonzeroDeltaCount(); - assert(count == 0); - delta = manager.currencyDelta(address(this), currency); - assert(delta == 0); - } - - return ""; - } -} - -contract SimpleLinearLocker is ILockCallback { - function(uint256) external checker; - - function main(IPoolManager manager, uint256 timesToReenter, function(uint256) external checker_) external { - checker = checker_; - manager.lock(address(this), abi.encode(timesToReenter, 0)); - } - - function lockAcquired(address, bytes calldata data) external returns (bytes memory) { - (uint256 timesToReenter, uint256 depth) = abi.decode(data, (uint256, uint256)); - checker(depth); - if (depth < timesToReenter) { - IPoolManager manager = IPoolManager(msg.sender); - manager.lock(address(this), abi.encode(timesToReenter, depth + 1)); - } - return ""; - } -} - -contract ParallelLocker is ILockCallback { - // We define an INDEX_OFFSET at 1 since the first locker is placed at index 1. - // The 0th index is used for storing the length. - uint256 constant INDEX_OFFSET = 1; - IPoolManager manager; - - constructor(IPoolManager manager_) { - manager = manager_; - } - - function main() external { - manager.lock(address(this), ""); - } - - function assertionChecker0(uint256) external view { - uint256 length = manager.getLockLength(); - assert(length == 2); - (address locker,) = manager.getLock(INDEX_OFFSET + 1); - assert(locker == msg.sender); - } - - function assertionChecker1(uint256 depth) external view { - uint256 length = manager.getLockLength(); - assert(length == depth + 2); - (address locker,) = manager.getLock(INDEX_OFFSET + depth + 1); - assert(locker == msg.sender); - } - - function assertionChecker2(uint256) external view { - uint256 length = manager.getLockLength(); - assert(length == 2); - (address locker,) = manager.getLock(INDEX_OFFSET + 1); - assert(locker == msg.sender); - } - - function lockAcquired(address, bytes calldata) external returns (bytes memory) { - SimpleLinearLocker locker0 = new SimpleLinearLocker(); - SimpleLinearLocker locker1 = new SimpleLinearLocker(); - SimpleLinearLocker locker2 = new SimpleLinearLocker(); - - uint256 length = manager.getLockLength(); - assert(length == 1); - (address locker,) = manager.getLock(INDEX_OFFSET + 0); - assert(locker == address(this)); - - locker0.main(manager, 0, this.assertionChecker0); - uint256 length1 = manager.getLockLength(); - assert(length1 == 1); - - locker1.main(manager, 1, this.assertionChecker1); - uint256 length2 = manager.getLockLength(); - assert(length2 == 1); - - locker2.main(manager, 0, this.assertionChecker2); - uint256 length3 = manager.getLockLength(); - assert(length3 == 1); - - return ""; - } -} - -contract PoolManagerReentrancyTest is Test, Deployers { - uint256 constant INDEX_OFFSET = 1; - - function setUp() public { - initializeManagerRoutersAndPoolsWithLiq(IHooks(address(0))); - } - - function testTokenLocker() public { - TokenLocker locker = new TokenLocker(); - MockERC20(Currency.unwrap(currency0)).mint(address(locker), 1); - MockERC20(Currency.unwrap(currency0)).approve(address(locker), 1); - locker.main(manager, currency0, true); - } - - function testTokenRevert() public { - TokenLocker locker = new TokenLocker(); - MockERC20(Currency.unwrap(currency0)).mint(address(locker), 1); - MockERC20(Currency.unwrap(currency0)).approve(address(locker), 1); - vm.expectRevert(abi.encodeWithSelector(IPoolManager.CurrencyNotSettled.selector)); - locker.main(manager, currency0, false); - } - - function assertionChecker(uint256 depth) external { - uint256 length = manager.getLockLength(); - assertEq(length, depth + 1); - (address locker,) = manager.getLock(INDEX_OFFSET + depth); - assertEq(locker, msg.sender); - } - - function testSimpleLinearLocker() public { - SimpleLinearLocker locker = new SimpleLinearLocker(); - locker.main(manager, 0, this.assertionChecker); - locker.main(manager, 1, this.assertionChecker); - locker.main(manager, 2, this.assertionChecker); - } - - function testParallelLocker() public { - ParallelLocker locker = new ParallelLocker(manager); - locker.main(); - } -} diff --git a/test/utils/Constants.sol b/test/utils/Constants.sol index 354a2d394..5df565aac 100644 --- a/test/utils/Constants.sol +++ b/test/utils/Constants.sol @@ -13,10 +13,16 @@ library Constants { uint128 constant MAX_UINT128 = type(uint128).max; uint160 constant MAX_UINT160 = type(uint160).max; + address constant ADDRESS_ZERO = address(0); + address constant EMPTY_HOOKS = address(0xf000000000000000000000000000000000000000); + address constant MOCK_HOOKS = address(0xfF00000000000000000000000000000000000000); + uint256 constant POOL_SLOT = 10; uint256 constant TICKS_OFFSET = 4; uint24 constant FEE_LOW = 500; uint24 constant FEE_MEDIUM = 3000; uint24 constant FEE_HIGH = 10000; + + bytes constant ZERO_BYTES = new bytes(0); } diff --git a/test/utils/Deployers.sol b/test/utils/Deployers.sol index e182cd5c7..d3fbc1658 100644 --- a/test/utils/Deployers.sol +++ b/test/utils/Deployers.sol @@ -18,6 +18,7 @@ import {PoolModifyLiquidityTest} from "../../src/test/PoolModifyLiquidityTest.so import {PoolSwapTest} from "../../src/test/PoolSwapTest.sol"; import {PoolInitializeTest} from "../../src/test/PoolInitializeTest.sol"; import {PoolDonateTest} from "../../src/test/PoolDonateTest.sol"; +import {PoolNestedActionsTest} from "../../src/test/PoolNestedActionsTest.sol"; import {PoolTakeTest} from "../../src/test/PoolTakeTest.sol"; import {PoolClaimsTest} from "../../src/test/PoolClaimsTest.sol"; import { @@ -34,7 +35,7 @@ contract Deployers { using CurrencyLibrary for Currency; // Helpful test constants - bytes constant ZERO_BYTES = new bytes(0); + bytes constant ZERO_BYTES = Constants.ZERO_BYTES; uint160 constant SQRT_RATIO_1_1 = Constants.SQRT_RATIO_1_1; uint160 constant SQRT_RATIO_1_2 = Constants.SQRT_RATIO_1_2; uint160 constant SQRT_RATIO_2_1 = Constants.SQRT_RATIO_2_1; @@ -59,6 +60,7 @@ contract Deployers { PoolTakeTest takeRouter; PoolClaimsTest claimsRouter; PoolInitializeTest initializeRouter; + PoolNestedActionsTest nestedActionRouter; ProtocolFeeControllerTest feeController; RevertingProtocolFeeControllerTest revertingFeeController; OutOfBoundsProtocolFeeControllerTest outOfBoundsFeeController; @@ -82,6 +84,7 @@ contract Deployers { takeRouter = new PoolTakeTest(manager); claimsRouter = new PoolClaimsTest(manager); initializeRouter = new PoolInitializeTest(manager); + nestedActionRouter = new PoolNestedActionsTest(manager); feeController = new ProtocolFeeControllerTest(); revertingFeeController = new RevertingProtocolFeeControllerTest(); outOfBoundsFeeController = new OutOfBoundsProtocolFeeControllerTest(); @@ -91,16 +94,19 @@ contract Deployers { manager.setProtocolFeeController(feeController); } + // You must have first initialised the routers with deployFreshManagerAndRouters + // If you only need the currencies (and not approvals) call deployAndMint2Currencies function deployMintAndApprove2Currencies() internal returns (Currency, Currency) { MockERC20[] memory tokens = deployTokens(2, 2 ** 255); - address[6] memory toApprove = [ + address[7] memory toApprove = [ address(swapRouter), address(modifyLiquidityRouter), address(donateRouter), address(takeRouter), address(claimsRouter), - address(initializeRouter) + address(initializeRouter), + address(nestedActionRouter.executor()) ]; for (uint256 i = 0; i < toApprove.length; i++) { @@ -112,6 +118,11 @@ contract Deployers { return (currency0, currency1); } + function deployAndMint2Currencies() internal returns (Currency, Currency) { + MockERC20[] memory tokens = deployTokens(2, 2 ** 255); + return SortTokens.sort(tokens[0], tokens[1]); + } + function deployTokens(uint8 count, uint256 totalSupply) internal returns (MockERC20[] memory tokens) { tokens = new MockERC20[](count); for (uint8 i = 0; i < count; i++) { @@ -164,6 +175,7 @@ contract Deployers { // sets the global currencyies and key deployMintAndApprove2Currencies(); (key,) = initPoolAndAddLiquidity(currency0, currency1, hooks, 3000, SQRT_RATIO_1_1, ZERO_BYTES); + nestedActionRouter.executor().setKey(key); (nativeKey,) = initPoolAndAddLiquidityETH( CurrencyLibrary.NATIVE, currency1, hooks, 3000, SQRT_RATIO_1_1, ZERO_BYTES, 1 ether ); diff --git a/test/utils/SwapHelper.t.sol b/test/utils/SwapHelper.t.sol index 64fd4990c..e89950c72 100644 --- a/test/utils/SwapHelper.t.sol +++ b/test/utils/SwapHelper.t.sol @@ -18,7 +18,6 @@ import {Deployers} from "./Deployers.sol"; import {Fees} from "../../src/Fees.sol"; import {PoolId, PoolIdLibrary} from "../../src/types/PoolId.sol"; import {PoolKey} from "../../src/types/PoolKey.sol"; -import {AccessLockHook} from "../../src/test/AccessLockHook.sol"; import {IERC20Minimal} from "../../src/interfaces/external/IERC20Minimal.sol"; import {BalanceDelta} from "../../src/types/BalanceDelta.sol";