Skip to content

Commit 5abe322

Browse files
mgretzkeclaude
andcommitted
fix: various comments addressed
- reorder param structs statics-first (ABI change) - inline _positionAmounts into the sweep step - reject pools whose hook carries a returns-delta permission upfront (UnsupportedHookPermissions) instead of documenting them only - move the trim cap into getLiquidityToFree with a forward round-down pre-check, making the previously documented far-edge overflow in the inverse structurally unreachable Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0da496e commit 5abe322

5 files changed

Lines changed: 133 additions & 87 deletions

File tree

src/SwapAndAdd.sol

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
pragma solidity 0.8.26;
33

44
import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
5+
import {IHooks} from "@uniswap/v4-core/src/interfaces/IHooks.sol";
56
import {ERC20} from "solmate/src/tokens/ERC20.sol";
67
import {ERC721} from "solmate/src/tokens/ERC721.sol";
78
import {SafeTransferLib} from "solmate/src/utils/SafeTransferLib.sol";
@@ -13,6 +14,7 @@ import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol";
1314
import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol";
1415
import {TransientStateLibrary} from "@uniswap/v4-core/src/libraries/TransientStateLibrary.sol";
1516
import {TickMath} from "@uniswap/v4-core/src/libraries/TickMath.sol";
17+
import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol";
1618

1719
import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol";
1820

@@ -47,7 +49,13 @@ contract SwapAndAdd is ISwapAndAdd, SafeCallback, DeltaResolver, Permit2Forwarde
4749
using PoolIdLibrary for PoolKey;
4850
using PositionInfoLibrary for PositionInfo;
4951
using SafeCast for uint256;
52+
using Hooks for IHooks;
5053

54+
/// @dev Hook permissions that let a hook alter this contract's settlement deltas, breaking the
55+
/// conservation accounting the reconcile relies on. Pools carrying any of them are rejected.
56+
uint160 private constant UNSUPPORTED_HOOK_FLAGS = Hooks.BEFORE_SWAP_RETURNS_DELTA_FLAG
57+
| Hooks.AFTER_SWAP_RETURNS_DELTA_FLAG | Hooks.AFTER_ADD_LIQUIDITY_RETURNS_DELTA_FLAG
58+
| Hooks.AFTER_REMOVE_LIQUIDITY_RETURNS_DELTA_FLAG;
5159
/// @dev Standing Permit2 allowance expiration timestamp.
5260
uint48 private constant ALLOWANCE_EXPIRATION = type(uint48).max;
5361
/// @dev Universal Router command to sweep unspent native ETH.
@@ -287,6 +295,10 @@ contract SwapAndAdd is ISwapAndAdd, SafeCallback, DeltaResolver, Permit2Forwarde
287295
internal
288296
returns (uint256 tokenId, uint128 liquidity, uint256 amount0, uint256 amount1)
289297
{
298+
// Pure bitmask on the hook address; a benignly-behaving hook with the permission is still rejected —
299+
// the boundary is the capability, not observed behavior.
300+
if (cp.key.hooks.hasPermission(UNSUPPORTED_HOOK_FLAGS)) revert UnsupportedHookPermissions(cp.key.hooks);
301+
290302
_ensureApproved(cp.key.currency0);
291303
_ensureApproved(cp.key.currency1);
292304

@@ -319,8 +331,9 @@ contract SwapAndAdd is ISwapAndAdd, SafeCallback, DeltaResolver, Permit2Forwarde
319331
// 4. Slippage Floor: Enforce minimum liquidity threshold on final post-trim position.
320332
if (liquidity < cp.minLiquidity) revert InsufficientLiquidity(cp.minLiquidity, liquidity);
321333

322-
// 5. Sweep: Calculate final token composition and sweep leftover dust in both pool tokens to recipient.
323-
(amount0, amount1) = _positionAmounts(cp, liquidity, sqrtLower, sqrtUpper);
334+
// 5. Sweep: Calculate final position token amounts at live price and sweep leftover dust to recipient.
335+
(uint160 sqrtPriceX96,,,) = poolManager.getSlot0(cp.key.toId());
336+
(amount0, amount1) = SwapAndAddMath.getAmountsForLiquidity(sqrtPriceX96, sqrtLower, sqrtUpper, liquidity);
324337
_sweep(cp.key.currency0, cp.recipient);
325338
_sweep(cp.key.currency1, cp.recipient);
326339
}
@@ -412,23 +425,11 @@ contract SwapAndAdd is ISwapAndAdd, SafeCallback, DeltaResolver, Permit2Forwarde
412425
) internal returns (uint128 dl) {
413426
// Fresh price read: the reconcile swap (or a hook) moved the price since sizing.
414427
(uint160 sqrtPriceX96,,,) = poolManager.getSlot0(cp.key.toId());
415-
uint256 liquidityToFree =
416-
SwapAndAddMath.getLiquidityToFree(sqrtPriceX96, sqrtLower, sqrtUpper, deficitIsCurrency1, amountOut);
417-
// Cap trim at the liquidity added in this transaction.
418-
dl = liquidityToFree >= lopt ? lopt : uint128(liquidityToFree);
428+
// Capped at `lopt`, the liquidity added in this transaction.
429+
dl = SwapAndAddMath.getLiquidityToFree(sqrtPriceX96, sqrtLower, sqrtUpper, deficitIsCurrency1, amountOut, lopt);
419430
_decrease(cp.key, tokenId, dl, cp.hookData);
420431
}
421432

422-
/// @dev Calculates final position token amounts at the current pool price.
423-
function _positionAmounts(CoreParams memory cp, uint128 liquidity, uint160 sqrtLower, uint160 sqrtUpper)
424-
internal
425-
view
426-
returns (uint256 amount0, uint256 amount1)
427-
{
428-
(uint160 sqrtPriceX96,,,) = poolManager.getSlot0(cp.key.toId());
429-
(amount0, amount1) = SwapAndAddMath.getAmountsForLiquidity(sqrtPriceX96, sqrtLower, sqrtUpper, liquidity);
430-
}
431-
432433
// ───────────────────────────────────────────── POSM / pool actions ─────────────────────────────────────────────
433434

434435
/// @dev Deploys liquidity via POSM (MINT for new position, INCREASE for existing).

src/interfaces/ISwapAndAdd.sol

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ pragma solidity ^0.8.24;
33

44
import {Currency} from "@uniswap/v4-core/src/types/Currency.sol";
55
import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";
6+
import {IHooks} from "@uniswap/v4-core/src/interfaces/IHooks.sol";
67

78
import {IMulticall_v4} from "./IMulticall_v4.sol";
89

@@ -34,7 +35,7 @@ import {IMulticall_v4} from "./IMulticall_v4.sol";
3435
/// (new NFT, cash-out, swept dust) is forced to the position owner to prevent value redirection.
3536
/// - Unsupported Pools: Pools with hooks returning deltas (`BEFORE_SWAP_RETURNS_DELTA`, `AFTER_SWAP_RETURNS_DELTA`,
3637
/// `AFTER_REMOVE_LIQUIDITY_RETURNS_DELTA`, `AFTER_ADD_LIQUIDITY_RETURNS_DELTA`) break settlement conservation
37-
/// and are unsupported. Failures revert atomically; funds remain safe. Dynamic fee, gating, and oracle hooks are supported.
38+
/// and are rejected upfront with `UnsupportedHookPermissions`. Dynamic fee, gating, and oracle hooks are supported.
3839
/// - Known Limits: Fee-on-transfer and rebasing tokens are unsupported (atomic reverts or larger trim, never token loss).
3940
/// Budgets within the pool's ~1-wei mint/burn rounding toll cannot settle and revert `InsufficientLiquidity`.
4041
/// - HookData Reuse: The same `hookData` payload is passed to all hook callbacks in the operation; single-use
@@ -116,6 +117,9 @@ interface ISwapAndAdd is IMulticall_v4 {
116117
/// @notice Thrown when a `routeFunding` token is one of the pool currencies (use `amount0In`/`amount1In` instead).
117118
error InvalidFundingToken(Currency token);
118119

120+
/// @notice Thrown when the pool's hook carries a returns-delta permission (see the Unsupported Pools note).
121+
error UnsupportedHookPermissions(IHooks hooks);
122+
119123
/// @notice Represents a non-pool token amount pulled to fund an off-chain route.
120124
/// @param token The non-pool token address (or address(0) for native ETH).
121125
/// @param amount The amount to pull from caller via Permit2 (or expected msg.value for native ETH). A zero amount pulls nothing but sweeps unlisted donations of that token.
@@ -130,24 +134,24 @@ interface ISwapAndAdd is IMulticall_v4 {
130134
/// @param tickUpper Upper tick of the position range.
131135
/// @param amount0In Amount of token0 budget to pull from caller (can be 0).
132136
/// @param amount1In Amount of token1 budget to pull from caller (can be 0).
133-
/// @param route Encoded Universal Router commands and inputs (empty for pure same-pool zap). Must scope input amounts explicitly (never spend entire contract balance).
134-
/// @param routeFunding Optional non-pool tokens pulled to fund the route. Unused amounts are swept to `recipient`.
135137
/// @param minLiquidity Minimum liquidity required for the minted position (slippage floor).
136138
/// @param recipient Address that receives the minted position NFT and leftover dust.
137-
/// @param hookData Arbitrary data passed to pool hooks (reused across all callbacks).
138139
/// @param deadline Timestamp after which the transaction will revert.
140+
/// @param route Encoded Universal Router commands and inputs (empty for pure same-pool zap). Must scope input amounts explicitly (never spend entire contract balance).
141+
/// @param routeFunding Optional non-pool tokens pulled to fund the route. Unused amounts are swept to `recipient`.
142+
/// @param hookData Arbitrary data passed to pool hooks (reused across all callbacks).
139143
struct AddParams {
140144
PoolKey poolKey;
141145
int24 tickLower;
142146
int24 tickUpper;
143147
uint256 amount0In;
144148
uint256 amount1In;
145-
bytes route;
146-
TokenAmount[] routeFunding;
147149
uint256 minLiquidity;
148150
address recipient;
149-
bytes hookData;
150151
uint256 deadline;
152+
bytes route;
153+
TokenAmount[] routeFunding;
154+
bytes hookData;
151155
}
152156

153157
/// @notice Create a new v4 position from a one- or two-sided token budget in a single transaction.
@@ -165,22 +169,22 @@ interface ISwapAndAdd is IMulticall_v4 {
165169
/// @param tokenId Existing position ID to increase.
166170
/// @param amount0In Amount of token0 budget to pull from caller (can be 0). Accrued fees are collected and reinvested automatically.
167171
/// @param amount1In Amount of token1 budget to pull from caller (can be 0). Accrued fees are collected and reinvested automatically.
168-
/// @param route Encoded Universal Router commands and inputs (can be empty).
169-
/// @param routeFunding Optional non-pool tokens pulled to fund the route. Unused amounts are swept to `recipient`.
170172
/// @param minLiquidityAdded Minimum liquidity that must be added to the position (slippage floor).
171173
/// @param recipient Destination for swept dust. Forced to position owner if caller is an operator.
172-
/// @param hookData Arbitrary data passed to pool hooks.
173174
/// @param deadline Timestamp after which the transaction will revert.
175+
/// @param route Encoded Universal Router commands and inputs (can be empty).
176+
/// @param routeFunding Optional non-pool tokens pulled to fund the route. Unused amounts are swept to `recipient`.
177+
/// @param hookData Arbitrary data passed to pool hooks.
174178
struct IncreaseParams {
175179
uint256 tokenId;
176180
uint256 amount0In;
177181
uint256 amount1In;
178-
bytes route;
179-
TokenAmount[] routeFunding;
180182
uint256 minLiquidityAdded;
181183
address recipient;
182-
bytes hookData;
183184
uint256 deadline;
185+
bytes route;
186+
TokenAmount[] routeFunding;
187+
bytes hookData;
184188
}
185189

186190
/// @notice Top up an existing position with a one- or two-sided budget and reinvest accrued fees in a single transaction.
@@ -200,24 +204,24 @@ interface ISwapAndAdd is IMulticall_v4 {
200204
/// @param additional1 Signed delta for token1 with the same signed delta semantics.
201205
/// @param newTickLower Lower tick of the new position range.
202206
/// @param newTickUpper Upper tick of the new position range.
203-
/// @param route Encoded Universal Router commands and inputs (can be empty).
204-
/// @param routeFunding Optional non-pool tokens pulled to fund the route. Unused amounts are swept to `recipient`.
205207
/// @param minLiquidity Minimum liquidity required for the newly minted position (slippage floor).
206208
/// @param recipient Destination for the new position NFT, returned cash-out tokens, and dust. Forced to owner if caller is an operator.
207-
/// @param hookData Arbitrary data passed to pool hooks.
208209
/// @param deadline Timestamp after which the transaction will revert.
210+
/// @param route Encoded Universal Router commands and inputs (can be empty).
211+
/// @param routeFunding Optional non-pool tokens pulled to fund the route. Unused amounts are swept to `recipient`.
212+
/// @param hookData Arbitrary data passed to pool hooks.
209213
struct RebalanceParams {
210214
uint256 tokenId;
211215
int128 additional0;
212216
int128 additional1;
213217
int24 newTickLower;
214218
int24 newTickUpper;
215-
bytes route;
216-
TokenAmount[] routeFunding;
217219
uint256 minLiquidity;
218220
address recipient;
219-
bytes hookData;
220221
uint256 deadline;
222+
bytes route;
223+
TokenAmount[] routeFunding;
224+
bytes hookData;
221225
}
222226

223227
/// @notice Withdraw an existing position entirely and redeploy it into a new tick range, optionally adding or cashing out tokens.
@@ -234,18 +238,18 @@ interface ISwapAndAdd is IMulticall_v4 {
234238

235239
/// @notice Parameters for `compound`.
236240
/// @param tokenId Existing position ID whose accrued fees will be collected and reinvested.
237-
/// @param route Encoded Universal Router commands and inputs (can be empty).
238241
/// @param minLiquidityAdded Minimum liquidity that must be added from reinvested fees (slippage floor).
239242
/// @param recipient Destination for swept dust. Forced to position owner if caller is an operator.
240-
/// @param hookData Arbitrary data passed to pool hooks.
241243
/// @param deadline Timestamp after which the transaction will revert.
244+
/// @param route Encoded Universal Router commands and inputs (can be empty).
245+
/// @param hookData Arbitrary data passed to pool hooks.
242246
struct CompoundParams {
243247
uint256 tokenId;
244-
bytes route;
245248
uint256 minLiquidityAdded;
246249
address recipient;
247-
bytes hookData;
248250
uint256 deadline;
251+
bytes route;
252+
bytes hookData;
249253
}
250254

251255
/// @notice Collect accrued fees on a position and reinvest them back into the same position in a single transaction.

src/libraries/SwapAndAddMath.sol

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -126,32 +126,42 @@ library SwapAndAddMath {
126126
}
127127

128128
/// @notice Computes the liquidity to burn so that v4's rounded-DOWN return covers `amountToCover` of the
129-
/// deficit token.
129+
/// deficit token, capped at `maxLiquidity`.
130130
/// @dev Exact ceil inverse over `amountToCover + 1`: the nested-floor bound guarantees freed >= amountToCover.
131131
/// Assumes the price is not past the range's far side for the deficit token (SwapAndAdd's reconcile
132132
/// flow guarantees this); a price outside the near side clamps to the boundary.
133-
/// @return liquidityToFree The round-up liquidity to burn, uncapped — callers cap against the liquidity they added.
133+
/// @param maxLiquidity Cap on the result. A binding cap means burning even that much cannot cover the
134+
/// debt; the caller's settlement surfaces the shortfall.
135+
/// @return liquidityToFree The round-up liquidity to burn, capped at `maxLiquidity`.
134136
function getLiquidityToFree(
135137
uint160 sqrtPriceX96,
136138
uint160 sqrtPriceLowerX96,
137139
uint160 sqrtPriceUpperX96,
138140
bool deficitIsCurrency1,
139-
uint256 amountToCover
140-
) internal pure returns (uint256 liquidityToFree) {
141+
uint256 amountToCover,
142+
uint128 maxLiquidity
143+
) internal pure returns (uint128 liquidityToFree) {
144+
// Forward check with v4's own round-DOWN burn formula: when even burning `maxLiquidity` cannot cover
145+
// the debt, the cap binds and the inverse is skipped. This also bounds the inverse's inputs so its
146+
// mulDiv result provably fits uint256 (its result exceeds the cap exactly when the check trips).
147+
uint256 uncapped;
141148
if (deficitIsCurrency1) {
142149
// Token1 occupies [sqrtLower, min(price, sqrtUpper)]: amount1 = L * (hi - lo) / Q96.
143150
uint160 clampedUpper = sqrtPriceX96 < sqrtPriceUpperX96 ? sqrtPriceX96 : sqrtPriceUpperX96;
144-
liquidityToFree =
145-
FullMath.mulDivRoundingUp(amountToCover + 1, FixedPoint96.Q96, clampedUpper - sqrtPriceLowerX96);
151+
if (amountToCover >= SqrtPriceMath.getAmount1Delta(sqrtPriceLowerX96, clampedUpper, maxLiquidity, false)) {
152+
return maxLiquidity;
153+
}
154+
uncapped = FullMath.mulDivRoundingUp(amountToCover + 1, FixedPoint96.Q96, clampedUpper - sqrtPriceLowerX96);
146155
} else {
147156
// Token0 occupies [max(price, sqrtLower), sqrtUpper]: amount0 = L * Q96 * (hi - lo) / (hi * lo).
148157
uint160 clampedLower = sqrtPriceX96 > sqrtPriceLowerX96 ? sqrtPriceX96 : sqrtPriceLowerX96;
158+
if (amountToCover >= SqrtPriceMath.getAmount0Delta(clampedLower, sqrtPriceUpperX96, maxLiquidity, false)) {
159+
return maxLiquidity;
160+
}
149161
uint256 intermediate = FullMath.mulDivRoundingUp(clampedLower, sqrtPriceUpperX96, FixedPoint96.Q96);
150-
// Informational: At extreme prices where post-swap price is within sqrt-units of sqrtUpper with large
151-
// deficit remaining, this intermediate quotient can overflow uint256 and revert (self-inflicted, safe).
152-
liquidityToFree =
153-
FullMath.mulDivRoundingUp(amountToCover + 1, intermediate, sqrtPriceUpperX96 - clampedLower);
162+
uncapped = FullMath.mulDivRoundingUp(amountToCover + 1, intermediate, sqrtPriceUpperX96 - clampedLower);
154163
}
164+
liquidityToFree = uncapped >= maxLiquidity ? maxLiquidity : uint128(uncapped);
155165
}
156166

157167
/// @dev Values a token pair in the cheaper-token numeraire, weighting each side by its pips factor.

test/SwapAndAdd.t.sol

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,30 @@ contract SwapAndAddTest is PosmTestSetup {
597597
zap.add(p);
598598
}
599599

600+
/// @dev Returns-delta hooks break settlement conservation and are rejected upfront with a typed error —
601+
/// the boundary is enforced in code, not just documented. One case per permission flag, each paired
602+
/// with the base flag v4 requires alongside it.
603+
function test_add_returnsDeltaHook_revertsUnsupported() public {
604+
uint160[4] memory flagged = [
605+
Hooks.BEFORE_SWAP_FLAG | Hooks.BEFORE_SWAP_RETURNS_DELTA_FLAG,
606+
Hooks.AFTER_SWAP_FLAG | Hooks.AFTER_SWAP_RETURNS_DELTA_FLAG,
607+
Hooks.AFTER_ADD_LIQUIDITY_FLAG | Hooks.AFTER_ADD_LIQUIDITY_RETURNS_DELTA_FLAG,
608+
Hooks.AFTER_REMOVE_LIQUIDITY_FLAG | Hooks.AFTER_REMOVE_LIQUIDITY_RETURNS_DELTA_FLAG
609+
];
610+
ISwapAndAdd.AddParams memory p = _addParams(1e18, 1e18);
611+
for (uint256 i = 0; i < flagged.length; i++) {
612+
p.poolKey.hooks = IHooks(address(flagged[i]));
613+
vm.expectRevert(abi.encodeWithSelector(ISwapAndAdd.UnsupportedHookPermissions.selector, p.poolKey.hooks));
614+
zap.add(p);
615+
}
616+
617+
// Negative control: a hook WITHOUT returns-delta permissions passes the gate — the same call
618+
// proceeds into the flow and fails deeper on the (uninitialized) pool instead.
619+
p.poolKey.hooks = IHooks(address(uint160(Hooks.BEFORE_SWAP_FLAG)));
620+
vm.expectRevert(IPoolManager.PoolNotInitialized.selector);
621+
zap.add(p);
622+
}
623+
600624
function test_rebalance_revertsIfNotAuthorized() public {
601625
(uint256 tokenId,,,) = zap.add(_addParams(0, 10e18));
602626
// do NOT approve the zap; call from a stranger

0 commit comments

Comments
 (0)