diff --git a/src/PoolManager.sol b/src/PoolManager.sol index 92b8148ad..badac55e8 100644 --- a/src/PoolManager.sol +++ b/src/PoolManager.sol @@ -341,6 +341,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim _pools[id].setLPFee(newDynamicLPFee); } + // if settling native, integrators should still call `sync` first to avoid DoS attack vectors function _settle(address recipient) internal returns (uint256 paid) { Currency currency = CurrencyReserves.getSyncedCurrency(); diff --git a/src/interfaces/IHooks.sol b/src/interfaces/IHooks.sol index a541e2681..4acfe941a 100644 --- a/src/interfaces/IHooks.sol +++ b/src/interfaces/IHooks.sol @@ -78,7 +78,7 @@ interface IHooks { /// @param sender The initial msg.sender for the remove liquidity call /// @param key The key for the pool /// @param params The parameters for removing liquidity - /// @param delta The caller's balance delta after adding liquidity; the sum of principal delta, fees accrued, and hook delta + /// @param delta The caller's balance delta after removing liquidity; the sum of principal delta, fees accrued, and hook delta /// @param feesAccrued The fees accrued since the last time fees were collected from this position /// @param hookData Arbitrary data handed into the PoolManager by the liquidity provider to be be passed on to the hook /// @return bytes4 The function selector for the hook diff --git a/src/interfaces/IPoolManager.sol b/src/interfaces/IPoolManager.sol index 3269851d0..b8502c247 100644 --- a/src/interfaces/IPoolManager.sol +++ b/src/interfaces/IPoolManager.sol @@ -186,7 +186,8 @@ interface IPoolManager is IProtocolFees, IERC6909Claims, IExtsload, IExttload { function sync(Currency currency) external; /// @notice Called by the user to net out some value owed to the user - /// @dev Can also be used as a mechanism for _free_ flash loans + /// @dev Will revert if the requested amount is not available, consider using `mint` instead + /// @dev Can also be used as a mechanism for free flash loans /// @param currency The currency to withdraw from the pool manager /// @param to The address to withdraw to /// @param amount The amount of currency to withdraw diff --git a/src/libraries/Hooks.sol b/src/libraries/Hooks.sol index 2f02548f5..d7c9ed328 100644 --- a/src/libraries/Hooks.sol +++ b/src/libraries/Hooks.sol @@ -159,7 +159,7 @@ library Hooks { function callHookWithReturnDelta(IHooks self, bytes memory data, bool parseReturn) internal returns (int256) { bytes memory result = callHook(self, data); - // If this hook wasnt meant to return something, default to 0 delta + // If this hook wasn't meant to return something, default to 0 delta if (!parseReturn) return 0; // A length of 64 bytes is required to return a bytes4, and a 32 byte delta @@ -258,7 +258,8 @@ library Hooks { // A length of 96 bytes is required to return a bytes4, a 32 byte delta, and an LP fee if (result.length != 96) InvalidHookResponse.selector.revertWith(); - // dynamic fee pools that do not want to override the cache fee, return 0 otherwise they return a valid fee with the override flag + // dynamic fee pools that want to override the cache fee, return a valid fee with the override flag. If override flag + // is set but an invalid fee is returned, the transaction will revert. Otherwise the current LP fee will be used if (key.fee.isDynamicFee()) lpFeeOverride = result.parseFee(); // skip this logic for the case where the hook return is 0 diff --git a/src/libraries/LPFeeLibrary.sol b/src/libraries/LPFeeLibrary.sol index a2a732a81..b7e145332 100644 --- a/src/libraries/LPFeeLibrary.sol +++ b/src/libraries/LPFeeLibrary.sol @@ -11,7 +11,7 @@ library LPFeeLibrary { /// @notice Thrown when the static or dynamic fee on a pool exceeds 100%. error LPFeeTooLarge(uint24 fee); - /// @notice An lp fee of exactly 0b1000000... signals a dynamic fee pool. This isnt a valid static fee as it is > MAX_LP_FEE + /// @notice An lp fee of exactly 0b1000000... signals a dynamic fee pool. This isn't a valid static fee as it is > MAX_LP_FEE uint24 public constant DYNAMIC_FEE_FLAG = 0x800000; /// @notice the second bit of the fee returned by beforeSwap is used to signal if the stored LP fee should be overridden in this swap diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol index fc714f6d2..1dfb56cc9 100644 --- a/src/libraries/Pool.sol +++ b/src/libraries/Pool.sol @@ -535,7 +535,8 @@ library Pool { } } - // when the lower (upper) tick is crossed left to right (right to left), liquidity must be added (removed) + // when the lower (upper) tick is crossed left to right, liquidity must be added (removed) + // when the lower (upper) tick is crossed right to left, liquidity must be removed (added) int128 liquidityNet = upper ? liquidityNetBefore - liquidityDelta : liquidityNetBefore + liquidityDelta; assembly ("memory-safe") { // liquidityGrossAfter and liquidityNet are packed in the first slot of `info` @@ -561,6 +562,7 @@ library Pool { function tickSpacingToMaxLiquidityPerTick(int24 tickSpacing) internal pure returns (uint128 result) { // Equivalent to: // int24 minTick = (TickMath.MIN_TICK / tickSpacing); + // if (TickMath.MIN_TICK % tickSpacing != 0) minTick--; // int24 maxTick = (TickMath.MAX_TICK / tickSpacing); // uint24 numTicks = maxTick - minTick + 1; // return type(uint128).max / numTicks; diff --git a/src/libraries/UnsafeMath.sol b/src/libraries/UnsafeMath.sol index c3f23c83c..4c313952a 100644 --- a/src/libraries/UnsafeMath.sol +++ b/src/libraries/UnsafeMath.sol @@ -5,7 +5,7 @@ pragma solidity ^0.8.0; /// @notice Contains methods that perform common math functions but do not do any overflow or underflow checks library UnsafeMath { /// @notice Returns ceil(x / y) - /// @dev division by 0 has unspecified behavior, and must be checked externally + /// @dev division by 0 will return 0, and should be checked externally /// @param x The dividend /// @param y The divisor /// @return z The quotient, ceil(x / y) @@ -16,7 +16,7 @@ library UnsafeMath { } /// @notice Calculates floor(a×b÷denominator) - /// @dev division by 0 has unspecified behavior, and must be checked externally + /// @dev division by 0 will return 0, and should be checked externally /// @param a The multiplicand /// @param b The multiplier /// @param denominator The divisor diff --git a/test/CustomAccounting.t.sol b/test/CustomAccounting.t.sol index e4942dce3..4a06aed0e 100644 --- a/test/CustomAccounting.t.sol +++ b/test/CustomAccounting.t.sol @@ -254,7 +254,7 @@ contract CustomAccountingTest is Test, Deployers, GasSnapshot { "manager balance change exact input" ); - // exact output, where there isnt enough output reserves available to pay swap and hook + // exact output, where there isn't enough output reserves available to pay swap and hook } else if (!isExactIn && (hookDeltaSpecified + amountSpecified > maxPossibleOut_fuzz_test)) { // the hook will have taken hookDeltaSpecified of the maxPossibleOut assertEq(deltaSpecified, maxPossibleOut_fuzz_test - hookDeltaSpecified, "deltaSpecified exact output");