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 <[email protected]> * Update src/libraries/NonZeroDeltaCount.sol Co-authored-by: Sara Reynolds <[email protected]> * test renaming --------- Co-authored-by: Sara Reynolds <[email protected]>
Uniswap · Feb 29, 2024 · 6f5f3f2 · 6f5f3f2
1 parent 4a13732
commit 6f5f3f2
Show file tree

Hide file tree

Showing 59 changed files with 617 additions and 2,023 deletions.
diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap
@@ -1 +1 @@
-323570
+313034
diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap
@@ -1 +1 @@
-200465
+193189
diff --git a/.forge-snapshots/addLiquidity.snap b/.forge-snapshots/addLiquidity.snap
@@ -1 +1 @@
-200443
+193167
diff --git 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
+185454
diff --git a/.forge-snapshots/cached dynamic fee, no hooks.snap b/.forge-snapshots/cached dynamic fee, no hooks.snap
@@ -1 +1 @@
-146291
+139454
diff --git a/.forge-snapshots/donate gas with 1 token.snap b/.forge-snapshots/donate gas with 1 token.snap
@@ -1 +1 @@
-137497
+132530
diff --git a/.forge-snapshots/donate gas with 2 tokens.snap b/.forge-snapshots/donate gas with 2 tokens.snap
@@ -1 +1 @@
-184854
+177634
diff --git a/.forge-snapshots/gas overhead of no-op lock.snap b/.forge-snapshots/gas overhead of no-op lock.snap
@@ -1 +1 @@
-15181
+14810
diff --git a/.forge-snapshots/initialize.snap b/.forge-snapshots/initialize.snap
@@ -1 +1 @@
-74139
+72764
diff --git a/.forge-snapshots/mintWithEmptyHookEOAInitiated.snap b/.forge-snapshots/mintWithEmptyHookEOAInitiated.snap
@@ -1 +1 @@
-255434
+247151
diff --git a/.forge-snapshots/modify position with noop.snap b/.forge-snapshots/modify position with noop.snap
@@ -1 +1 @@
-58501
+54101
diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap
@@ -1 +1 @@
-24361
+23847
diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap
@@ -1 +1 @@
-107256
+99936
diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap
@@ -1 +1 @@
-208629
+201309
diff --git a/.forge-snapshots/removeLiquidity.snap b/.forge-snapshots/removeLiquidity.snap
@@ -1 +1 @@
-204906
+197586
diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap
@@ -1 +1 @@
-195291
+188027
diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap
@@ -1 +1 @@
-203828
+196564
diff --git a/.forge-snapshots/simpleSwapEOAInitiated.snap b/.forge-snapshots/simpleSwapEOAInitiated.snap
@@ -1 +1 @@
-173759
+166450
diff --git a/.forge-snapshots/simpleSwapNativeEOAInitiated.snap b/.forge-snapshots/simpleSwapNativeEOAInitiated.snap
@@ -1 +1 @@
-172441
+165132
diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap
@@ -1 +1 @@
-125954
+118690
diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap
@@ -1 +1 @@
-113418
+106154
diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap
@@ -1 +1 @@
-133630
+126344
diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap
@@ -1 +1 @@
-129583
+122297
diff --git a/.forge-snapshots/swap mint native output as 6909.snap b/.forge-snapshots/swap mint native output as 6909.snap
@@ -1 +1 @@
-197863
+190554
diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap
@@ -1 +1 @@
-214668
+207359
diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap
@@ -1 +1 @@
-193022
+184555
diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap
@@ -1 +1 @@
-113396
+106132
diff --git a/.forge-snapshots/swap with noop.snap b/.forge-snapshots/swap with noop.snap
@@ -1 +1 @@
-51199
+46833
diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap
@@ -1 +1 @@
-199770
+191259
diff --git 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
@@ -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);