general cleanup (#707)

* general cleanup

* remove empty lines

* revert rename of currency settle take

* PR comment fixes

src/PoolManager.sol

@@ -81,7 +81,6 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
     using Pool for *;
     using Hooks for IHooks;
     using Position for mapping(bytes32 => Position.Info);
-    using CurrencyLibrary for Currency;
     using CurrencyDelta for Currency;
     using LPFeeLibrary for uint24;
     using Reserves for Currency;
@@ -97,10 +96,6 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
 
     constructor(uint256 controllerGasLimit) ProtocolFees(controllerGasLimit) {}
 
-    function _getPool(PoolId id) internal view override returns (Pool.State storage) {
-        return _pools[id];
-    }
-
     /// @notice This will revert if the contract is locked
     modifier onlyWhenUnlocked() {
         if (!Lock.isUnlocked()) ManagerLocked.selector.revertWith();
@@ -154,31 +149,6 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
         currency.setReserves(balance);
     }
 
-    function _accountDelta(Currency currency, int128 delta, address target) internal {
-        if (delta == 0) return;
-
-        int256 current = currency.getDelta(target);
-        int256 next = current + delta;
-
-        if (next == 0) {
-            NonZeroDeltaCount.decrement();
-        } else if (current == 0) {
-            NonZeroDeltaCount.increment();
-        }
-
-        currency.setDelta(target, next);
-    }
-
-    /// @dev Accumulates a balance change to a map of currency to balance changes
-    function _accountPoolBalanceDelta(PoolKey memory key, BalanceDelta delta, address target) internal {
-        _accountDelta(key.currency0, delta.amount0(), target);
-        _accountDelta(key.currency1, delta.amount1(), target);
-    }
-
-    function _checkPoolInitialized(PoolId id) internal view {
-        if (_pools[id].isNotInitialized()) PoolNotInitialized.selector.revertWith();
-    }
-
     /// @inheritdoc IPoolManager
     function modifyLiquidity(
         PoolKey memory key,
@@ -256,7 +226,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
         _accountPoolBalanceDelta(key, swapDelta, msg.sender);
     }
 
-    // Internal swap function to execute a swap, take protocol fees on input token, and emit the swap event
+    /// @notice Internal swap function to execute a swap, take protocol fees on input token, and emit the swap event
     function _swap(PoolId id, Pool.SwapParams memory params, Currency inputCurrency) internal returns (BalanceDelta) {
         (BalanceDelta delta, uint256 feeForProtocol, uint24 swapFee, Pool.SwapState memory state) =
             _pools[id].swap(params);
@@ -328,6 +298,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
         _burnFrom(from, id, amount);
     }
 
+    /// @inheritdoc IPoolManager
     function updateDynamicLPFee(PoolKey memory key, uint24 newDynamicLPFee) external {
         if (!key.fee.isDynamicFee() || msg.sender != address(key.hooks)) {
             UnauthorizedDynamicLPFeeUpdate.selector.revertWith();
@@ -336,4 +307,36 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
         PoolId id = key.toId();
         _pools[id].setLPFee(newDynamicLPFee);
     }
+
+    /// @notice Adds a balance delta in a currency for a target address
+    function _accountDelta(Currency currency, int128 delta, address target) internal {
+        if (delta == 0) return;
+
+        int256 current = currency.getDelta(target);
+        int256 next = current + delta;
+
+        if (next == 0) {
+            NonZeroDeltaCount.decrement();
+        } else if (current == 0) {
+            NonZeroDeltaCount.increment();
+        }
+
+        currency.setDelta(target, next);
+    }
+
+    /// @notice Accounts the deltas of 2 currencies to a target address
+    function _accountPoolBalanceDelta(PoolKey memory key, BalanceDelta delta, address target) internal {
+        _accountDelta(key.currency0, delta.amount0(), target);
+        _accountDelta(key.currency1, delta.amount1(), target);
+    }
+
+    /// @notice Checks if a given pool has been initialized
+    function _checkPoolInitialized(PoolId id) internal view {
+        if (_pools[id].isNotInitialized()) PoolNotInitialized.selector.revertWith();
+    }
+
+    /// @notice implementation of the _getPool function defined in ProtocolFees
+    function _getPool(PoolId id) internal view override returns (Pool.State storage) {
+        return _pools[id];
+    }
 }

src/ProtocolFees.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 pragma solidity ^0.8.19;
 
-import {Currency, CurrencyLibrary} from "./types/Currency.sol";
+import {Currency} from "./types/Currency.sol";
 import {IProtocolFeeController} from "./interfaces/IProtocolFeeController.sol";
 import {IProtocolFees} from "./interfaces/IProtocolFees.sol";
 import {PoolKey} from "./types/PoolKey.sol";
@@ -12,7 +12,6 @@ import {Pool} from "./libraries/Pool.sol";
 import {CustomRevert} from "./libraries/CustomRevert.sol";
 
 abstract contract ProtocolFees is IProtocolFees, Owned {
-    using CurrencyLibrary for Currency;
     using ProtocolFeeLibrary for uint24;
     using PoolIdLibrary for PoolKey;
     using Pool for Pool.State;
@@ -55,6 +54,8 @@ abstract contract ProtocolFees is IProtocolFees, Owned {
         currency.transfer(recipient, amountCollected);
     }
 
+    /// @dev abstract internal function to allow the ProtocolFees contract to access pool state
+    /// @dev this is overriden in PoolManager.sol to give access to the _pools mapping
     function _getPool(PoolId id) internal virtual returns (Pool.State storage);
 
     /// @notice Fetch the protocol fees for a given pool, returning false if the call fails or the returned fees are invalid.

src/libraries/CurrencyDelta.sol

@@ -3,7 +3,10 @@ pragma solidity ^0.8.20;
 
 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) {
         assembly {
             mstore(0, caller_)
@@ -12,6 +15,7 @@ library CurrencyDelta {
         }
     }
 
+    /// @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);
 
@@ -20,6 +24,7 @@ library CurrencyDelta {
         }
     }
 
+    /// @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);
 

src/libraries/LPFeeLibrary.sol

@@ -11,31 +11,36 @@ library LPFeeLibrary {
     /// @notice Thrown when the static or dynamic fee on a pool exceeds 100%.
     error FeeTooLarge();
 
-    uint24 public constant FEE_MASK = 0x7FFFFF;
-    uint24 public constant OVERRIDE_MASK = 0xBFFFFF;
-
     // the top bit of the fee in a PoolKey is used to signal if a Pool's LP fee is dynamic
     uint24 public constant DYNAMIC_FEE_FLAG = 0x800000;
 
     // the second bit of the fee returned by beforeSwap is used to signal if the stored LP fee should be overridden in this swap
     // only dynamic-fee pools can return a fee via the beforeSwap hook
     uint24 public constant OVERRIDE_FEE_FLAG = 0x400000;
 
+    // mask to remove the override fee flag from a fee returned by the beforeSwaphook
+    uint24 public constant REMOVE_OVERRIDE_MASK = 0xBFFFFF;
+
     // the lp fee is represented in hundredths of a bip, so the max is 100%
     uint24 public constant MAX_LP_FEE = 1000000;
 
+    /// @notice returns true if a pool's LP fee signals that the pool has a dynamic fee
     function isDynamicFee(uint24 self) internal pure returns (bool) {
         return self & DYNAMIC_FEE_FLAG != 0;
     }
 
+    /// @notice returns true if an LP fee is valid, aka not above the maxmimum permitted fee
     function isValid(uint24 self) internal pure returns (bool) {
         return self <= MAX_LP_FEE;
     }
 
+    /// @notice validates whether an LP fee is larger than the maximum, and reverts if invalid
     function validate(uint24 self) internal pure {
         if (!self.isValid()) FeeTooLarge.selector.revertWith();
     }
 
+    /// @notice gets and validates the initial LP fee for a pool. Dynamic fee pools have an initial fee of 0.
+    /// @dev if a dynamic fee pool wants a non-0 initial fee, it should call `updateDynamicLPFee` in the afterInitialize hook
     function getInitialLPFee(uint24 self) internal pure returns (uint24) {
         // the initial fee for a dynamic fee pool is 0
         if (self.isDynamicFee()) return 0;
@@ -50,11 +55,11 @@ library LPFeeLibrary {
 
     /// @notice returns a fee with the override flag removed
     function removeOverrideFlag(uint24 self) internal pure returns (uint24) {
-        return self & OVERRIDE_MASK;
+        return self & REMOVE_OVERRIDE_MASK;
     }
 
     /// @notice Removes the override flag and validates the fee (reverts if the fee is too large)
-    function removeOverrideAndValidate(uint24 self) internal pure returns (uint24 fee) {
+    function removeOverrideFlagAndValidate(uint24 self) internal pure returns (uint24 fee) {
         fee = self.removeOverrideFlag();
         fee.validate();
     }

src/libraries/Pool.sol

@@ -305,7 +305,7 @@ library Pool {
         // if the beforeSwap hook returned a valid fee override, use that as the LP fee, otherwise load from storage
         {
             uint24 lpFee = params.lpFeeOverride.isOverride()
-                ? params.lpFeeOverride.removeOverrideAndValidate()
+                ? params.lpFeeOverride.removeOverrideFlagAndValidate()
                 : slot0Start.lpFee();
 
             swapFee = protocolFee == 0 ? lpFee : uint24(protocolFee).calculateSwapFee(lpFee);

src/test/ActionsRouter.sol

@@ -4,7 +4,7 @@ pragma solidity ^0.8.24;
 import "forge-std/Test.sol";
 import {IPoolManager} from "../interfaces/IPoolManager.sol";
 import {IUnlockCallback} from "../interfaces/callback/IUnlockCallback.sol";
-import {Currency, CurrencyLibrary} from "../types/Currency.sol";
+import {Currency} from "../types/Currency.sol";
 import {PoolKey} from "../types/PoolKey.sol";
 import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol";
 import {StateLibrary} from "../libraries/StateLibrary.sol";
@@ -30,7 +30,6 @@ enum Actions {
 /// @notice A router that handles an arbitrary input of actions.
 /// TODO: Can continue to add functions per action.
 contract ActionsRouter is IUnlockCallback, Test {
-    using CurrencyLibrary for Currency;
     using StateLibrary for IPoolManager;
     using TransientStateLibrary for IPoolManager;
 

src/test/CurrencyTest.sol

@@ -5,23 +5,23 @@ import {Currency, CurrencyLibrary} from "../types/Currency.sol";
 
 contract CurrencyTest {
     function transfer(Currency currency, address to, uint256 amount) external {
-        CurrencyLibrary.transfer(currency, to, amount);
+        currency.transfer(to, amount);
     }
 
     function balanceOfSelf(Currency currency) external view returns (uint256) {
-        return CurrencyLibrary.balanceOfSelf(currency);
+        return currency.balanceOfSelf();
     }
 
     function balanceOf(Currency currency, address owner) external view returns (uint256) {
-        return CurrencyLibrary.balanceOf(currency, owner);
+        return currency.balanceOf(owner);
     }
 
     function isNative(Currency currency) external pure returns (bool) {
-        return CurrencyLibrary.isNative(currency);
+        return currency.isNative();
     }
 
     function toId(Currency currency) external pure returns (uint256) {
-        return CurrencyLibrary.toId(currency);
+        return currency.toId();
     }
 
     function fromId(uint256 id) external pure returns (Currency) {

src/test/CustomCurveHook.sol

@@ -12,11 +12,10 @@ import {Currency} from "../types/Currency.sol";
 import {CurrencySettler} from "../../test/utils/CurrencySettler.sol";
 import {BaseTestHooks} from "./BaseTestHooks.sol";
 import {IERC20Minimal} from "../interfaces/external/IERC20Minimal.sol";
-import {CurrencyLibrary, Currency} from "../types/Currency.sol";
+import {Currency} from "../types/Currency.sol";
 
 contract CustomCurveHook is BaseTestHooks {
     using Hooks for IHooks;
-    using CurrencyLibrary for Currency;
     using CurrencySettler for Currency;
 
     error AddLiquidityDirectToHook();

src/test/DeltaReturningHook.sol

@@ -11,12 +11,11 @@ import {BalanceDelta, toBalanceDelta} from "../types/BalanceDelta.sol";
 import {Currency} from "../types/Currency.sol";
 import {BaseTestHooks} from "./BaseTestHooks.sol";
 import {IERC20Minimal} from "../interfaces/external/IERC20Minimal.sol";
-import {CurrencyLibrary, Currency} from "../types/Currency.sol";
+import {Currency} from "../types/Currency.sol";
 import {BeforeSwapDelta, toBeforeSwapDelta} from "../types/BeforeSwapDelta.sol";
 
 contract DeltaReturningHook is BaseTestHooks {
     using Hooks for IHooks;
-    using CurrencyLibrary for Currency;
     using CurrencySettler for Currency;
 
     IPoolManager immutable manager;

src/test/MockERC6909Claims.sol

@@ -3,12 +3,10 @@ pragma solidity ^0.8.20;
 
 import {Test} from "forge-std/Test.sol";
 import {ERC6909Claims} from "../ERC6909Claims.sol";
-import {CurrencyLibrary, Currency} from "../types/Currency.sol";
+import {Currency} from "../types/Currency.sol";
 
 /// @notice Mock contract for testing ERC6909Claims
 contract MockERC6909Claims is ERC6909Claims {
-    using CurrencyLibrary for Currency;
-
     /// @notice mocked mint logic
     function mint(address to, uint256 id, uint256 amount) public {
         _mint(to, id, amount);

src/test/PoolClaimsTest.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.24;
 
-import {Currency, CurrencyLibrary} from "../types/Currency.sol";
+import {Currency} from "../types/Currency.sol";
 import {BalanceDelta, toBalanceDelta} from "../types/BalanceDelta.sol";
 import {IPoolManager} from "../interfaces/IPoolManager.sol";
 import {PoolKey} from "../types/PoolKey.sol";
@@ -10,7 +10,6 @@ import {SafeCast} from "../libraries/SafeCast.sol";
 import {CurrencySettler} from "../../test/utils/CurrencySettler.sol";
 
 contract PoolClaimsTest is PoolTestBase {
-    using CurrencyLibrary for Currency;
     using CurrencySettler for Currency;
     using SafeCast for uint256;
 

src/test/PoolDonateTest.sol

@@ -11,7 +11,6 @@ import {Hooks} from "../libraries/Hooks.sol";
 import {CurrencySettler} from "../../test/utils/CurrencySettler.sol";
 
 contract PoolDonateTest is PoolTestBase {
-    using CurrencyLibrary for Currency;
     using CurrencySettler for Currency;
     using Hooks for IHooks;
 

src/test/PoolModifyLiquidityTest.sol

@@ -14,7 +14,6 @@ import {CurrencySettler} from "../../test/utils/CurrencySettler.sol";
 import {StateLibrary} from "../libraries/StateLibrary.sol";
 
 contract PoolModifyLiquidityTest is PoolTestBase {
-    using CurrencyLibrary for Currency;
     using CurrencySettler for Currency;
     using Hooks for IHooks;
     using LPFeeLibrary for uint24;

src/test/PoolModifyLiquidityTestNoChecks.sol

@@ -14,7 +14,6 @@ import {CurrencySettler} from "../../test/utils/CurrencySettler.sol";
 import {Constants} from "../../test/utils/Constants.sol";
 
 contract PoolModifyLiquidityTestNoChecks is PoolTestBase {
-    using CurrencyLibrary for Currency;
     using CurrencySettler for Currency;
     using Hooks for IHooks;
     using LPFeeLibrary for uint24;

src/test/PoolSwapTest.sol

@@ -11,7 +11,6 @@ import {PoolTestBase} from "./PoolTestBase.sol";
 import {CurrencySettler} from "../../test/utils/CurrencySettler.sol";
 
 contract PoolSwapTest is PoolTestBase {
-    using CurrencyLibrary for Currency;
     using CurrencySettler for Currency;
     using Hooks for IHooks;
 

src/test/PoolTakeTest.sol

@@ -1,15 +1,14 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.24;
 
-import {Currency, CurrencyLibrary} from "../types/Currency.sol";
+import {Currency} from "../types/Currency.sol";
 import {IPoolManager} from "../interfaces/IPoolManager.sol";
 import {PoolKey} from "../types/PoolKey.sol";
 import {PoolTestBase} from "./PoolTestBase.sol";
 import {SafeCast} from "../libraries/SafeCast.sol";
 import {CurrencySettler} from "../../test/utils/CurrencySettler.sol";
 
 contract PoolTakeTest is PoolTestBase {
-    using CurrencyLibrary for Currency;
     using CurrencySettler for Currency;
     using SafeCast for uint256;
 

src/test/PoolTestBase.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.24;
 
-import {CurrencyLibrary, Currency} from "../types/Currency.sol";
+import {Currency} from "../types/Currency.sol";
 import {IERC20Minimal} from "../interfaces/external/IERC20Minimal.sol";
 
 import {IUnlockCallback} from "../interfaces/callback/IUnlockCallback.sol";
@@ -11,7 +11,6 @@ import {StateLibrary} from "../libraries/StateLibrary.sol";
 import {TransientStateLibrary} from "../libraries/TransientStateLibrary.sol";
 
 abstract contract PoolTestBase is IUnlockCallback {
-    using CurrencyLibrary for Currency;
     using StateLibrary for IPoolManager;
     using TransientStateLibrary for IPoolManager;
 

src/test/SkipCallsTestHook.sol

@@ -9,7 +9,7 @@ import {PoolKey} from "../types/PoolKey.sol";
 import {BalanceDelta, BalanceDeltaLibrary} from "../types/BalanceDelta.sol";
 import {PoolId, PoolIdLibrary} from "../types/PoolId.sol";
 import {IERC20Minimal} from "../interfaces/external/IERC20Minimal.sol";
-import {CurrencyLibrary, Currency} from "../types/Currency.sol";
+import {Currency} from "../types/Currency.sol";
 import {PoolTestBase} from "./PoolTestBase.sol";
 import {Constants} from "../../test/utils/Constants.sol";
 import {Test} from "forge-std/Test.sol";