Fix #752: Reuse _computeSlot result in _accountDelta (#753)

* fix #752 * forge snapshot --isolate * forge fmt * add the requested natspec * caller -> target * consistent capitalization * rename updated to next --------- Co-authored-by: Alice <[email protected]>
Uniswap · Jul 15, 2024 · 1f4e8b6 · 1f4e8b6
1 parent 87292df
commit 1f4e8b6
Show file tree

Hide file tree

Showing 34 changed files with 50 additions and 53 deletions.
diff --git a/.forge-snapshots/add liquidity to already existing position with salt.snap b/.forge-snapshots/add liquidity to already existing position with salt.snap
@@ -1 +1 @@
-146557
+146297
diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap
@@ -1 +1 @@
-172605
+172085
diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap
@@ -1 +1 @@
-275714
+275454
diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap
@@ -1 +1 @@
-136837
+136577
diff --git a/.forge-snapshots/create new liquidity to a position with salt.snap b/.forge-snapshots/create new liquidity to a position with salt.snap
@@ -1 +1 @@
-294762
+294502
diff --git a/.forge-snapshots/donate gas with 1 token.snap b/.forge-snapshots/donate gas with 1 token.snap
@@ -1 +1 @@
-104674
+104544
diff --git a/.forge-snapshots/donate gas with 2 tokens.snap b/.forge-snapshots/donate gas with 2 tokens.snap
@@ -1 +1 @@
-144554
+144294
diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap
@@ -1 +1 @@
-23821
+23809
diff --git a/.forge-snapshots/removeLiquidity CA fee.snap b/.forge-snapshots/removeLiquidity CA fee.snap
@@ -1 +1 @@
-142125
+141605
diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap
@@ -1 +1 @@
-131597
+131337
diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap
@@ -1 +1 @@
-113398
+113190
diff --git a/.forge-snapshots/simple addLiquidity second addition same range.snap b/.forge-snapshots/simple addLiquidity second addition same range.snap
@@ -1 +1 @@
-98747
+98617
diff --git a/.forge-snapshots/simple addLiquidity.snap b/.forge-snapshots/simple addLiquidity.snap
@@ -1 +1 @@
-161227
+161097
diff --git a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap
@@ -1 +1 @@
-92503
+92373
diff --git a/.forge-snapshots/simple removeLiquidity.snap b/.forge-snapshots/simple removeLiquidity.snap
@@ -1 +1 @@
-84560
+84430
diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap
@@ -1 +1 @@
-109712
+109452
diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap
@@ -1 +1 @@
-124538
+124278
diff --git a/.forge-snapshots/swap CA custom curve + swap noop.snap b/.forge-snapshots/swap CA custom curve + swap noop.snap
@@ -1 +1 @@
-127257
+126737
diff --git a/.forge-snapshots/swap CA fee on unspecified.snap b/.forge-snapshots/swap CA fee on unspecified.snap
@@ -1 +1 @@
-155691
+155301
diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap
@@ -1 +1 @@
-106660
+106400
diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap
@@ -1 +1 @@
-117746
+117486
diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap
@@ -1 +1 @@
-129595
+129335
diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap
@@ -1 +1 @@
-118816
+118556
diff --git a/.forge-snapshots/swap mint native output as 6909.snap b/.forge-snapshots/swap mint native output as 6909.snap
@@ -1 +1 @@
-140629
+140369
diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap
@@ -1 +1 @@
-156433
+156173
diff --git a/.forge-snapshots/swap skips hook call if hook is caller.snap b/.forge-snapshots/swap skips hook call if hook is caller.snap
@@ -1 +1 @@
-208547
+208027
diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap
@@ -1 +1 @@
-140527
+140267
diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap
@@ -1 +1 @@
-133197
+132937
diff --git a/.forge-snapshots/swap with lp fee and protocol fee.snap b/.forge-snapshots/swap with lp fee and protocol fee.snap
@@ -1 +1 @@
-170330
+170070
diff --git a/.forge-snapshots/swap with return dynamic fee.snap b/.forge-snapshots/swap with return dynamic fee.snap
@@ -1 +1 @@
-146758
+146498
diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap
@@ -1 +1 @@
-149016
+148756
diff --git a/src/PoolManager.sol b/src/PoolManager.sol
@@ -323,16 +323,13 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
     function _accountDelta(Currency currency, int128 delta, address target) internal {
         if (delta == 0) return;
 
-        int256 current = currency.getDelta(target);
-        int256 next = current + delta;
+        (int256 previous, int256 next) = currency.applyDelta(target, delta);
 
         if (next == 0) {
             NonZeroDeltaCount.decrement();
-        } else if (current == 0) {
+        } else if (previous == 0) {
             NonZeroDeltaCount.increment();
         }
-
-        currency.setDelta(target, next);
     }
 
     /// @notice Accounts the deltas of 2 currencies to a target address

diff --git a/src/libraries/CurrencyDelta.sol b/src/libraries/CurrencyDelta.sol
@@ -6,30 +6,30 @@ import {Currency} from "../types/Currency.sol";
 /// @title a library to store callers' currency deltas in transient storage
 /// @dev this library implements the equivalent of a mapping, as transient storage can only be accessed in assembly
 library CurrencyDelta {
-    /// @notice calculates which storage slot a delta should be stored in for a given caller and currency
-    function _computeSlot(address caller_, Currency currency) internal pure returns (bytes32 hashSlot) {
+    /// @notice calculates which storage slot a delta should be stored in for a given account and currency
+    function _computeSlot(address target, Currency currency) internal pure returns (bytes32 hashSlot) {
         assembly ("memory-safe") {
-            mstore(0, caller_)
+            mstore(0, target)
             mstore(32, currency)
             hashSlot := keccak256(0, 64)
         }
     }
 
-    /// @notice sets a new currency delta for a given caller and currency
-    function setDelta(Currency currency, address caller, int256 delta) internal {
-        bytes32 hashSlot = _computeSlot(caller, currency);
+    /// @notice applies a new currency delta for a given account and currency
+    /// @return previous The prior value
+    /// @return next The modified result
+    function applyDelta(Currency currency, address target, int128 delta)
+        internal
+        returns (int256 previous, int256 next)
+    {
+        bytes32 hashSlot = _computeSlot(target, currency);
 
         assembly {
-            tstore(hashSlot, delta)
+            previous := tload(hashSlot)
         }
-    }
-
-    /// @notice gets a new currency delta for a given caller and currency
-    function getDelta(Currency currency, address caller) internal view returns (int256 delta) {
-        bytes32 hashSlot = _computeSlot(caller, currency);
-
+        next = previous + delta;
         assembly {
-            delta := tload(hashSlot)
+            tstore(hashSlot, next)
         }
     }
 }
diff --git a/src/libraries/TransientStateLibrary.sol b/src/libraries/TransientStateLibrary.sol
@@ -39,12 +39,12 @@ library TransientStateLibrary {
     }
 
     /// @notice Get the current delta for a caller in the given currency
-    /// @param caller_ The address of the caller
+    /// @param target The credited account address
     /// @param currency The currency for which to lookup the delta
-    function currencyDelta(IPoolManager manager, address caller_, Currency currency) internal view returns (int256) {
+    function currencyDelta(IPoolManager manager, address target, Currency currency) internal view returns (int256) {
         bytes32 key;
         assembly ("memory-safe") {
-            mstore(0, caller_)
+            mstore(0, target)
             mstore(32, currency)
             key := keccak256(0, 64)
         }