Nonzero delta count capitalization (#815)

* make capitalization of NonzeroDeltaCount more consistent * rename files * rename NonZeroNativeValue -> NonzeroNativeValue * rename test * fix merge overwrites
Uniswap · Aug 14, 2024 · 3609d81 · 3609d81
1 parent e37e010
commit 3609d81
Show file tree

Hide file tree

Showing 10 changed files with 63 additions and 63 deletions.
diff --git a/src/PoolManager.sol b/src/PoolManager.sol
@@ -20,7 +20,7 @@ import {BalanceDelta, BalanceDeltaLibrary} from "./types/BalanceDelta.sol";
 import {BeforeSwapDelta} from "./types/BeforeSwapDelta.sol";
 import {Lock} from "./libraries/Lock.sol";
 import {CurrencyDelta} from "./libraries/CurrencyDelta.sol";
-import {NonZeroDeltaCount} from "./libraries/NonZeroDeltaCount.sol";
+import {NonzeroDeltaCount} from "./libraries/NonzeroDeltaCount.sol";
 import {CurrencyReserves} from "./libraries/CurrencyReserves.sol";
 import {Extsload} from "./Extsload.sol";
 import {Exttload} from "./Exttload.sol";
@@ -110,7 +110,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
         // the caller does everything in this callback, including paying what they owe via calls to settle
         result = IUnlockCallback(msg.sender).unlockCallback(data);
 
-        if (NonZeroDeltaCount.read() != 0) CurrencyNotSettled.selector.revertWith();
+        if (NonzeroDeltaCount.read() != 0) CurrencyNotSettled.selector.revertWith();
         Lock.lock();
     }
 
@@ -334,7 +334,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
         if (currency.isNative()) {
             paid = msg.value;
         } else {
-            if (msg.value > 0) NonZeroNativeValue.selector.revertWith();
+            if (msg.value > 0) NonzeroNativeValue.selector.revertWith();
             // Reserves are guaranteed to be set, because currency and reserves are always set together
             uint256 reservesBefore = CurrencyReserves.getSyncedReserves();
             uint256 reservesNow = currency.balanceOfSelf();
@@ -351,9 +351,9 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
         (int256 previous, int256 next) = currency.applyDelta(target, delta);
 
         if (next == 0) {
-            NonZeroDeltaCount.decrement();
+            NonzeroDeltaCount.decrement();
         } else if (previous == 0) {
-            NonZeroDeltaCount.increment();
+            NonzeroDeltaCount.increment();
         }
     }
 

diff --git a/src/interfaces/IPoolManager.sol b/src/interfaces/IPoolManager.sol
@@ -42,7 +42,7 @@ interface IPoolManager is IProtocolFees, IERC6909Claims, IExtsload, IExttload {
     error SwapAmountCannotBeZero();
 
     ///@notice Thrown when native currency is passed to a non native settlement
-    error NonZeroNativeValue();
+    error NonzeroNativeValue();
 
     /// @notice Thrown when `clear` is called with an amount that is not exactly equal to the open currency delta.
     error MustClearExactPositiveDelta();

diff --git a/src/libraries/NonZeroDeltaCount.sol → src/libraries/NonzeroDeltaCount.sol b/src/libraries/NonZeroDeltaCount.sol → src/libraries/NonzeroDeltaCount.sol
@@ -4,7 +4,7 @@ pragma solidity ^0.8.24;
 /// @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 {
+library NonzeroDeltaCount {
     // The slot holding the number of nonzero deltas. bytes32(uint256(keccak256("NonzeroDeltaCount")) - 1)
     bytes32 internal constant NONZERO_DELTA_COUNT_SLOT =
         0x7d4b3164c6e45b97e7d87b7125a44c5828d005af88f9d751cfd78729c5d99a0b;

diff --git a/src/libraries/TransientStateLibrary.sol b/src/libraries/TransientStateLibrary.sol
@@ -4,7 +4,7 @@ pragma solidity ^0.8.24;
 import {IPoolManager} from "../interfaces/IPoolManager.sol";
 import {Currency} from "../types/Currency.sol";
 import {CurrencyReserves} from "./CurrencyReserves.sol";
-import {NonZeroDeltaCount} from "./NonZeroDeltaCount.sol";
+import {NonzeroDeltaCount} from "./NonzeroDeltaCount.sol";
 import {Lock} from "./Lock.sol";
 
 /// @notice A helper library to provide state getters that use exttload
@@ -26,7 +26,7 @@ library TransientStateLibrary {
 
     /// @notice Returns the number of nonzero deltas open on the PoolManager that must be zerod out before the contract is locked
     function getNonzeroDeltaCount(IPoolManager manager) internal view returns (uint256) {
-        return uint256(manager.exttload(NonZeroDeltaCount.NONZERO_DELTA_COUNT_SLOT));
+        return uint256(manager.exttload(NonzeroDeltaCount.NONZERO_DELTA_COUNT_SLOT));
     }
 
     /// @notice Get the current delta for a caller in the given currency

diff --git a/src/test/ProxyPoolManager.sol b/src/test/ProxyPoolManager.sol
@@ -20,7 +20,7 @@ import {BalanceDelta, BalanceDeltaLibrary, toBalanceDelta} from "../types/Balanc
 import {BeforeSwapDelta} from "../types/BeforeSwapDelta.sol";
 import {Lock} from "../libraries/Lock.sol";
 import {CurrencyDelta} from "../libraries/CurrencyDelta.sol";
-import {NonZeroDeltaCount} from "../libraries/NonZeroDeltaCount.sol";
+import {NonzeroDeltaCount} from "../libraries/NonzeroDeltaCount.sol";
 import {CurrencyReserves} from "../libraries/CurrencyReserves.sol";
 import {Extsload} from "../Extsload.sol";
 import {Exttload} from "../Exttload.sol";
@@ -65,7 +65,7 @@ contract ProxyPoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909
         // the caller does everything in this callback, including paying what they owe via calls to settle
         result = IUnlockCallback(msg.sender).unlockCallback(data);
 
-        if (NonZeroDeltaCount.read() != 0) CurrencyNotSettled.selector.revertWith();
+        if (NonzeroDeltaCount.read() != 0) CurrencyNotSettled.selector.revertWith();
         Lock.lock();
     }
 

diff --git a/test/PoolManager.clear.t.sol b/test/PoolManager.clear.t.sol
@@ -134,7 +134,7 @@ contract ClearTest is Test, Deployers {
         actionsRouter.executeActions(actions, params);
     }
 
-    function test_clear_reverts_zeroDelta_inputNonZero() external {
+    function test_clear_reverts_zeroDelta_inputNonzero() external {
         uint256 amount = 1e18;
         Actions[] memory actions = new Actions[](3);
         bytes[] memory params = new bytes[](3);
@@ -145,7 +145,7 @@ contract ClearTest is Test, Deployers {
         actions[1] = Actions.ASSERT_DELTA_EQUALS;
         params[1] = abi.encode(currency0, address(actionsRouter), 0);
 
-        // Clear with nonZero.
+        // Clear with nonzero.
         actions[2] = Actions.CLEAR;
         params[2] = abi.encode(currency0, amount, false, "");
 

diff --git a/test/PoolManager.t.sol b/test/PoolManager.t.sol
@@ -934,7 +934,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
 
     function test_settle_revertsSendingNativeWithToken() public noIsolate {
         manager.sync(key.currency0);
-        vm.expectRevert(IPoolManager.NonZeroNativeValue.selector);
+        vm.expectRevert(IPoolManager.NonzeroNativeValue.selector);
         settleRouter.settle{value: 1}();
     }
 

diff --git a/test/Sync.t.sol b/test/Sync.t.sol
@@ -43,7 +43,7 @@ contract SyncTest is Test, Deployers, GasSnapshot {
         assertEq(manager.getSyncedReserves(), 0);
     }
 
-    function test_sync_balanceIsNonZero() public noIsolate {
+    function test_sync_balanceIsNonzero() public noIsolate {
         uint256 currency0Balance = currency0.balanceOf(address(manager));
         assertGt(currency0Balance, uint256(0));
 
@@ -218,11 +218,11 @@ contract SyncTest is Test, Deployers, GasSnapshot {
         Actions[] memory actions = new Actions[](1);
         bytes[] memory params = new bytes[](1);
 
-        // Revert with NonZeroNativeValue
+        // Revert with NonzeroNativeValue
         actions[0] = Actions.SETTLE_NATIVE;
         params[0] = abi.encode(value);
 
-        vm.expectRevert(IPoolManager.NonZeroNativeValue.selector);
+        vm.expectRevert(IPoolManager.NonzeroNativeValue.selector);
         router.executeActions{value: value}(actions, params);
 
         // Reference only - see OZ C01 report - previous test confirming vulnerability

diff --git a/test/libraries/NonZeroDeltaCount.t.sol b/test/libraries/NonZeroDeltaCount.t.sol
diff --git a/test/libraries/NonzeroDeltaCount.t.sol b/test/libraries/NonzeroDeltaCount.t.sol
@@ -0,0 +1,46 @@
+// 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 {
+    function test_incrementNonzeroDeltaCount() public {
+        assertEq(NonzeroDeltaCount.read(), 0);
+        NonzeroDeltaCount.increment();
+        assertEq(NonzeroDeltaCount.read(), 1);
+    }
+
+    function test_decrementNonzeroDeltaCount() public {
+        assertEq(NonzeroDeltaCount.read(), 0);
+        NonzeroDeltaCount.increment();
+        assertEq(NonzeroDeltaCount.read(), 1);
+        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 {
+        assertEq(NonzeroDeltaCount.read(), 0);
+        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);
+        }
+    }
+
+    function test_nonzeroDeltaCountSlot() public pure {
+        assertEq(bytes32(uint256(keccak256("NonzeroDeltaCount")) - 1), NonzeroDeltaCount.NONZERO_DELTA_COUNT_SLOT);
+    }
+}