feat: SwapAndAdd — (add / rebalance / increase / compound) by mgretzke · Pull Request #591 · Uniswap/v4-periphery

mgretzke · 2026-08-25T16:49:13Z

Summary

SwapAndAdd lets a caller supply the two pool tokens in any ratio — including only one, or an unrelated token routed through the Universal Router — and end up with a standard POSM ERC-721 position in a single transaction. Four operations share one core:

add — fresh capital → new position
rebalance — burn an existing position → new position in a new range, with signed per-token add/cash-out deltas
increase — fresh capital + the position's accrued fees → grow an existing tokenId in place
compound — accrued fees only (an increase with a zero pulled budget)

Design

Route first, then size from reality. The naive "compute the exact swap for the perfect ratio" is a circular equation (the swap moves the price that defines the ratio). Instead: run the caller's verbatim UR route first (a black box — it may even trade the target pool), size the position from the actual post-route balances at the live price (fee-aware: the side the same-pool reconcile will sell is discounted by that direction's total swap fee), mint optimistically by flash-taking the shortfall from the PoolManager, then one same-pool swap plus a bounded trim make the books balance. minLiquidity on the final, post-trim position is the single slippage knob — no swap-rate inputs, no per-amount minimums.

One main unlock per operation (_run → _unlockCallback → _swapAndAdd); rebalance first burns the old position through POSM's own vanilla burn path so every op enters the funnel with resolved budgets. The unlock payload is the CoreParams struct itself.

No funds at rest. Every operation pulls, deploys/settles in full, and sweeps the remainder (pool-token dust — possibly in both tokens, never re-denominated — and unconsumed route funding) within the same operation. Standing max Permit2 allowances to POSM and the UR are safe under this invariant: those spenders only ever pull from their direct caller, i.e. only mid-operation, under the reentrancy lock, with minLiquidity bounding the outcome.

Multicall (POSM-style) enables [permitBatch, op] — approve by signature and operate in one transaction — and keeper batches over several positions. msg.value keeps exactly one meaning per batch: ops assert exact equality plus a balance guard, and native spending is balance-funded, so the classic double-spend is unrepresentable (test-pinned).

Auth model. Rebalance/increase/compound require owner or ERC-721-approved operator; an operator's entire output (NFT, cash-out, dust, unconsumed funding) is forced to the owner, so a standing approval can never redirect position value. Op-level events (Added/Rebalanced/Increased/Compounded) index recipient and tokenIds — Rebalanced carries the old→new lineage nothing onchain otherwise links.

Known limits (documented in `ISwapAndAdd`)

Hooks with *_RETURNS_DELTA permissions are unsupported (they break the reconcile's conservation identity); failure is an atomic revert, funds safe. Dynamic LP-fee hooks are supported and tested.
Wei-scale budgets can't fully settle the pool's mint/burn rounding toll; a non-zero floor surfaces this as InsufficientLiquidity.
Fee-on-transfer and rebasing tokens are unsupported (as pool currencies — v4 itself does not support them — and as route funding); failures are atomic reverts or a larger trim.
POSM subscribers are notified from inside the operation; a reverting or pool-trading subscriber affects only its own position's operations, gated by the floor (test-pinned).

Testing

815 tests green under --isolate with gas-snapshot check:

unit suites per surface (core ops, trim, route funding, same-pool routes, out-of-range increases, subscribers, constructor, events)
pool-config property fuzz (price ±300k ticks, fee, spacing, depth, ranges, budgets): success + no-funds-at-rest or a whitelisted clean revert
math fuzzes held at 5000 runs: trim-inverse never under-frees across the full sqrt-price domain; fee-aware sizing accuracy via a dust-tightness bound proven to fail if the discount breaks; extreme-tick full-deployment in both numeraires
adversarial multicall: batch-vs-sequential observational equivalence (fuzzed), atomic rollback, cross-owner permit mixing, operator batches, msg.value discipline
a far-edge probe pinning that the reconcile sell cannot reach the range's far side on supported pools (grid + fuzz)
fork suites (Sepolia live-bytecode + mainnet Trading-API fixture) live in test-integration/ under the integration profile

Deployment note

The routed path requires the Universal Router feature that runs V4_SWAP inside an existing PoolManager unlock (submodule pinned at cf27fb66e5; upstream candidate for the next UR release). Until that ships in the canonical router, a deployment pairs the zap with its own UR build — the deploy script supports reusing one via ROUTER=. Contract compiles under the POSM optimizer profile (via_ir, 500 runs) at 18,233 bytes runtime.

🤖 Generated with Claude Code

…tive Standalone v4 swap-and-add/rebalance (ISwapAndAdd + SwapAndAdd). Mint-first single-mint flow: size L on-chain from the budget at a conservative swap rate, flash-take the deficit, mint directly to the user via POSM modifyLiquiditiesWithoutUnlock, source the deficit (verbatim UR route + same-pool reconcile), settle, sweep leftover (input token only). Single-token or any-ratio budget; native ETH; rebalance (full+partial) reuses the add core via withdraw->add. Slippage = single on-chain minLiquidity floor; bounded per-call Permit2 allowance to UR/POSM; no meaningful B-dust. Tests (test/SwapAndAdd.t.sol, 8 passing, same-pool path): single-token both directions, any-ratio, native ETH, rebalance full/partial, minLiquidity + auth reverts. Deferred (documented): UR-route integration test needs the modified UR from universal-router@feat/v4-swap-within-existing-unlock; V2 threshold dust-deploy. Note: swapRateX96 is the conservative token1-per-token0 sizing rate; off-chain must supply it in the conservative direction for the trade. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…integration Rewrites SwapAndAdd to option C: optimistically size the position from the full budget at the live slot0 mid price, mint to the contract, fund the deficit via the verbatim UR route + same-pool swap, then DECREASE (trim) the position by exactly the residual shortfall and transfer the NFT to the recipient post-unlock. Single minLiquidity floor on the FINAL post-trim position; swapRateX96 removed. ERC20 + native ETH; add + rebalance (currently liquidityToMove/option-b semantics). Deploys the actual max (dust ~= realized slippage, in the input token) vs the old conservative size. Adds the real-UR route integration: universal-router pulled in as a submodule pinned to cf27fb6 (feat/v4-swap-within-existing-unlock), a [profile.integration] in foundry.toml, and test-integration/SwapAndAddRoute.t.sol exercising a V4_SWAP route within the zap's own unlock. test/MintTrimProbe.t.sol documents the same-unlock mint->swap->decrease primitive. Tests: 9 SwapAndAddTest pass (default profile); SwapAndAddRouteTest passes under FOUNDRY_PROFILE=integration. Not pushed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

… v4 + real UR Forks mainnet (publicnode RPC), deploys the modified UR (mainnet RouterParameters) + SwapAndAdd against the real PoolManager/POSM/Permit2, and exercises the real ETH/USDC v4 pool 0xdce6...f78d (native ETH / USDC / 3000 / 60 / no hook). 3 tests pass: USDC-budget empty-route add, native-ETH-budget empty-route add, and USDC-budget add via a real V4_SWAP route through the real UR within the zap's unlock. FOUNDRY_PROFILE=integration. Not pushed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…lance-via-route Unit (13 pass): adds deadline reverts (add/rebalance), InvalidEthValue on the native path, and a realistic minLiquidity-floor revert (snapshot realized L, re-run with minLiquidity=L+1 so the trim lands one wei under the floor -> InsufficientLiquidity). Integration (7 pass): fork rebalance full/partial on the real ETH/USDC pool (option-b: remainder stays in the old range), and rebalance whose surplus->deficit leg runs through the real UR within the unlock. No contract issues surfaced. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ce revert) _getAmountsForLiquidity rounded DOWN while POSM's MINT_POSITION pulls round-UP, so when the rounded-down required amount landed == budget, _executeMint's flash-take guard didn't fire and the mint pulled 1 wei more than held → TRANSFER_FROM_FAILED. Systematic trigger: full same-range (and near-balanced) rebalance. Fix: round the estimate UP (4x getAmount*Delta false→true) so it equals POSM's actual pull; the flash-take then tops up correctly. Safe for sizing (round-up only shrinks L); <=1-wei over-estimate is swept. Adds same-range regression tests (full USDC + 0.5-ETH repro). 13 unit + 9 integration green. Note: a same-range full rebalance is never exactly liquidity-preserving (round-up sizing + trim cost ⇒ marginally less L), so a zero-haircut minLiquidity floor will revert InsufficientLiquidity — the UI's slippage haircut covers this. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Rewrites the zap to route-first: run the verbatim UR route FIRST (off-venue), size the position from the actual post-route holdings (fee-aware: discount the side the same-pool reconcile will swap by lpFee+protocolFee), mint to the contract, reconcile the residual with one bidirectional same-pool swap + trim, enforce the single minLiquidity floor, sweep, transfer NFT post-unlock. Chosen via the empirical bake-off (decision doc in scratchpad): route-first deploys the most liquidity in every routed regime (cheaper-route +0.12-0.47%, better-than-mid +1-2.5%, worse-than-mid +0.86%), ties no-route/neutral, and minLiquidity gives both architectures identical loss-protection. Interface unchanged (fees read on-chain; minLiquidity is the one knob). Adds a 'why route before mint' doc section (the circularity-break) + per-step NatSpec. Tests: 17 unit (incl. better-than-mid capture, cheaper-route deploys-more, route over/under-converts, + all prior invariants/reverts/native/rebalance/same-range) and 10 integration vs the REAL modified UR (route-within-unlock add+rebalance; fork add/rebalance/same-range + a route-first low-dust fork test) — all green. Gas ~+13k vs mint-first on empty-route, accepted for the deployment gain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…t to wallet) Switches rebalance from option-b (partial decrease, remainder left in old range) to option-a: always BURN the full old position, redeploy a caller-chosen fraction (redeployBps, 0<bps<=10000) of the withdrawn value into the new range, and return the rest to the recipient's wallet. Rationale: rebalances are usually triggered by an out-of-range position, whose un-moved liquidity would sit idle in the dead range — returning it to the wallet is more useful, and matches the UI's 'add back to position %' slider. Interface: RebalanceParams.liquidityToMove(uint128) -> redeployBps(uint256); added InvalidRedeployBps. Accounting: the full position is burned to real held balances (TAKE_PAIR, no open delta), then the (1-bps) excess is transferred to the recipient UP FRONT — so every balanceOfSelf read in the route-first add flow sees only the redeploy share. bps=10000 == old full rebalance. Tests: 18 unit (partial-redeploy burns-full+returns-rest, InvalidBps revert, full/auth/deadline) + 10 integration incl. fork partial-redeploy on the real ETH/USDC pool — both profiles green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Feature 1 — RebalanceParams.redeployBps -> int128 additionalA/additionalB (signed per-token deltas applied to the full-burned holdings): positive pulls from the wallet (rebalance + add, in one tx), negative returns to the wallet (cash-out, clamped to withdrawn via ReturnExceedsWithdrawn), (0,0) = full redeploy. Generalizes the old bps knob, removes rounding, allows mixed signs. Positives pulled in the entrypoint (_pullAdditional, msg.sender=caller, exact msg.value); negatives returned up front (_resolveBudget); _addCore reused unchanged. Removed redeployBps/_returnExcess/BPS_DENOMINATOR/InvalidRedeployBps. Feature 2 — compound(CompoundParams): reinvest a position's accrued fees back into the SAME tokenId, fees never touching the wallet. Collect via DECREASE_LIQUIDITY(0)+TAKE -> fee-aware size (_planMint) -> flash-take -> new INCREASE_LIQUIDITY (_increase/_buildIncreaseParams, mirrors _mint incl. native SWEEP) -> reconcile+trim -> minLiquidityAdded floor -> sweep. NFT never moves; reverts NoFeesToCompound when nothing accrued. Auth = ERC721-approved (same onlyIfApproved-locker path as the existing burn/decrease). Factored _checkAuth + _flashTakeDeficit for reuse. Tests: 26 unit + 13 integration (real UR) green — signed-delta rebalance (full/cash-out/add-more/mixed/over-withdraw-revert/native + wrong-msg.value), compound (reinvests fees, NFT stays, floor + auth + no-fees reverts, fork real-fee generation). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…self An ERC-721 approved operator may rebalance or compound the owner's position, but must never be able to redirect its value to themselves. Honor a custom `recipient` only when the caller is the position owner; for an approved operator, force all output (new NFT, negative-delta cash-out, swept dust) to the owner. This makes a standing NFT approval safe to keep — an operator can manage the position but can never steal it. Token (Permit2) pulls were already msg.sender-only, so wallet funds were never at risk. Adds guard tests: operator-cannot-redirect (rebalance + compound) and owner-may-choose-recipient. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

New `increase(IncreaseParams)` entrypoint: deposit a one- or two-sided budget into an EXISTING position, in place, in one tx. Reuses the route-first core — `_addCore(cp, existingTokenId)` now serves add (mint, tokenId 0), increase, and compound; increase just passes the tokenId so the core INCREASEs instead of MINTing. No new NFT, the NFT never moves, funds pull from msg.sender. POSM gates INCREASE_LIQUIDITY on the contract being approved (onlyIfApproved), same trust model as compound/rebalance; no caller-auth or value-redirect path. 6 new unit tests (grows-same-position, mixed/single/native budgets, minLiquidity + deadline reverts); 35 unit + 13 integration green. Note: at the repo default optimizer_runs the runtime exceeds 24576 bytes — a production deploy needs SwapAndAdd pinned to optimizer_runs=500 (~20.3KB), handled as a follow-up build-config change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

At the repo default optimizer_runs (44.4M, tuned for runtime gas) SwapAndAdd's runtime is ~26.9KB — over the EIP-170 24,576 limit. Pin it to the existing `posm` profile (via_ir=true, optimizer_runs=500) → ~20.5KB, deployable, and consistent with how PositionManager/PositionDescriptor are handled. The unit test imported the concrete `SwapAndAdd` + `new SwapAndAdd(...)`, which dragged the via_ir=true-restricted source into the via_ir=false `test/**` unit and made it unsatisfiable. Mirror the POSM pattern: interact via `ISwapAndAdd` and deploy with `deployCode("SwapAndAdd.sol:SwapAndAdd", ...)` from the precompiled artifact, so the concrete source never enters the via_ir=false unit. 35 unit + 13 integration tests green; deployedBytecode 20,543 bytes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Behavioral fixes (each with a regression test): - size fee-aware math from the DIRECTIONAL protocol fee via ProtocolFeeLibrary.calculateSwapFee; the packed Slot0.protocolFee was previously added to lpFee as plain pips, underflowing sizing on any pool with a protocol fee set - fund the route on BOTH sides and let it declare its own input; reclaim unconsumed native from the Universal Router via a SWEEP execute (UR balances are permissionlessly sweepable) - clamp the native value forwarded to POSM to the held balance (ranges entirely below spot hold no buffer wei and reverted OutOfFunds) - cap the reconcile trim at the liquidity the operation just added, so increase/compound can never consume the owner's pre-existing principal - approve Permit2 with SafeTransferLib.safeApprove: tokens whose approve returns nothing (USDT) reverted the plain IERC20 call on decode, bricking every pool of that token - widen int128 negation in _resolveBudget (type(int128).min) Restructure: - merge _mint/_increase and their param builders into one _deploy - one-time standing max Permit2 allowances to POSM/UR (_ensureApproved), justified by the documented no-funds-at-rest contract invariant - restrict receive() to PoolManager/POSM/UR (InvalidEthSender) - checkDeadline modifier; _authAndResolveRecipient merges the auth check with the operator recipient-forcing rule - normative NatSpec: operator TRUST NOTE, fee-taking-hook KNOWN LIMIT, route funding semantics Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

- regression tests: directional protocol fee, native below-range mint, approve-no-return (USDT-style) token, native partial-route-value reclaimed from the UR (integration) - promote the review probes into the suite proper: thin-pool extremes, narrow-range huge single-sided, price-at-range-edge, two-sided fuzz - remove the temporary probe harnesses (MintTrimProbe, SwapAndAddProbe); the mechanics they spiked are covered by the real suite - mainnet fork suite: FORK_BLOCK env pins a reproducible block when an archive RPC is available (keyless default stays at head) - foundry.toml: integration-profile comment updated for the split test-integration/ layout Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Scope + architecture + trust assumptions (incl. the unmerged universal-router branch dependency), the normative invariants (no-funds-at-rest, trim cap, operator recipient forcing, single slippage gate), known issues / accepted risks, slither triage, and build/test instructions for the audit handoff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The shipped design diverged (mint-to-contract + trim and route-first were out of scope there); the normative docs now live in ISwapAndAdd's NatSpec and audits/SwapAndAdd-audit-scope.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The NatSpec claimed the route "does not touch this pool" as if it were a mechanism guarantee. Nothing enforces that: correctness comes from sizing AFTER the route, from re-read balances at the then-live price — whatever the route did (even to this pool), the post-route state is the source of truth and minLiquidity remains the single gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Compound was the only op without the route leg, on the grounds that fees are small — but the execution cost of the same-pool reconcile is proportional to the compounded amount, i.e. a recurring haircut on yield itself. The shared core already runs routes, so CompoundParams gains `route` and _compound passes it through instead of hardcoding "". Sizing note (interface): quote the route input at the currently-unclaimed fees — fees only accrue upward, the reconcile absorbs the drift, and a competing collection reverts the pull atomically. Operator trust model unchanged (an approved operator can already collect fees via POSM directly; minLiquidityAdded bounds any route atomically). Tests: mock-route compound unit test; real-UR compound route integration test — also the first existing-tokenId path exercising the nested UR execute. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

… ERC721 POSM IS a solmate ERC721 (via ERC721Permit_v4) — its interface just doesn't declare that surface. Casting to the actual base type replaces the hand-rolled minimal interface; same pattern PermissionsAdapter uses with solmate ERC20. Identical bytecode size, no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The pool selects its amounts branch by TICK, the zap by SQRTPRICE; the only states where they can disagree are prices exactly on a tick boundary (a limit-clamped swap landing on an initialized tick stores tick = boundary - 1 zeroForOne / boundary oneForZero). Engineer both states, assert them, and prove the branch formulas coincide — a divergence would make POSM pull a token the zap never funded and revert the unlock. Also surfaced: the wei-level flash-take needs PM-WIDE reserves of the deficit token — the boundary swap drains the pool universe's entire token0, and the take reverts atomically until any position re-supplies it. Documented as K-05 (academic in production, funds safe). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…-exact The buffer guarded against POSM pulling more than the sized amounts, but that divergence is structurally impossible: _getAmountsForLiquidity rounds up with the pool's own SqrtPriceMath on the pool's own inputs, and the sole subtle case — price exactly on a tick boundary, where the pool branches on tick and the zap on sqrtPrice — degenerates to equal amounts (pinned by the test_add_priceExactlyOn* boundary tests; full suites pass with the buffer removed, 5000-run fuzz included). Removing it also deletes the _nativeToForward clamp the buffer had made necessary (its absence was a real OutOfFunds bug on below-range native mints), and turns every deficit-funded test into a standing wei-exact funding check. Audit doc D-04 rewritten accordingly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The trim only runs while the flash debt is unpaid, and that debt is the just-added range's own deficit side — the exact-input reconcile sell repays it in full at the latest when it exhausts the range, so the price can never be past the range's FAR side with deficit still owed. The near-side min/max clamps stay (reachable: a range fully beyond spot trims from a price outside it — now pinned by test_add_belowRange_singleToken0). If the invariant were ever violated the sorted liquidity math still yields a capped dl and the unlock reverts atomically, same as the fallbacks did. Also: document the K-05 no-pre-check decision on _flashTakeDeficit (a globally drained PM fails the take, but that state leaves the reconcile nothing to swap against either — the op is unviable regardless). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…val wiring Sizing (reported Medium): - REFERENCE_LIQUIDITY 1e18 -> uint128.max so the reference amounts sit far above their wei round-up (narrow ranges at extreme ticks deployed ~1%) - value in the CHEAPER token: the conversion rate is always >= Q96, so the token1-per-token0 rate no longer truncates to zero below tick ~-665455 - explicit PoolNotInitialized guard before the sizing division Approvals: - drop the _tokenApproved flag; detect wiring from live state (Permit2->POSM allowance as init marker, ERC20 headroom re-check as self-heal) with zero-first re-approve; handles Permit2-native and approve-race tokens Tests: extreme-tick pins + full-domain fuzz, weird-token approval mocks, native-as-currency1 revert pin. Doc polish across entrypoints and callback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

… reconcile swaps, dynamic-fee + ETH notes - reject recipient == address(this) on all four entrypoints (resolved recipient for rebalance/compound): a self-recipient would strand the minted NFT forever and break no-funds-at-rest (new InvalidRecipient) - forward the operation's hookData to the reconcile swaps instead of "": the payload was already presented to every liquidity callback (mint, trim, burn/collect), so this removes the one arbitrary exception; the interface documents that single-use/nonce payload schemes are unsupported (callback count is state-dependent, 0-2 swaps) - DYNAMIC-FEE NOTE: sizing uses the stored Slot0 fee; a beforeSwap override is charged at reconcile, absorbed by the trim, and floored by minLiquidity — pinned by MockDynamicFeeHook (strict hookData hash + 10% override vs stored 0) - ETH NOTE: value attached to the inherited payable permit calls is an unsolicited donation (Permit2Forwarder is shared/non-virtual; the zap has no multicall, so the payability serves nothing here) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

UniversalRouter (feat/v4-swap-within-existing-unlock @ cf27fb66e5): 0x44518461733Fd7f5DC5996facB405CF659108Ea2 SwapAndAdd: 0xc6b69cbB1f9EB78D15C3876105B9EDA458CB404F Block 11276910. The canonical UR cannot serve routed operations (V4_SWAP inside the zap's open unlock reverts AlreadyUnlocked), so each deployment carries its own router instance until the UR feature ships canonically. Wiring verified onchain (universalRouter/positionManager getters). Also: .env scaffolding (gitignored; keystore recommended for keys) and a live Sepolia RPC alias. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Binds to the deployed zap + patched UR (nothing re-deployed) and proves the live bytecode end-to-end: first-LP on an empty pool, single-sided reconcile, native budget, V4_SWAP route inside the zap's unlock on the LIVE router (the canonical UR would revert AlreadyUnlocked), rebalance, and compound of fees generated by real UR swaps. Pools are created in-fork so the suite stays deterministic regardless of Sepolia's ambient liquidity. Plus fetch-tapi-route.sh: turns a Trading API quote into the zap's route bytes (swapper must be the zap; drop the deadline; no permit data). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…mpatibility finding test_forkSepolia_add_viaRealTapiRoute replays verbatim TAPI /v1/swap calldata (swapper = the live zap) at a pinned block: 50e6 USDC -> WETH through the real, TAPI-indexed v4 USDC/WETH 0.01%/60 Sepolia pool, deployed into that same pool by the live zap + patched UR. FINDING pinned by the test: the UR feature branch added minHopPriceX36 to every IV4Router swap-param struct, so raw TAPI v4 legs revert on this deployment (v2/v3 legs are command-level and unaffected). The fixture transcodes canonical -> branch actions (_transcodeTapiV4Actions, hop price limits zeroed) and reverts loudly on unhandled actions. The transcoder must die when the branch restores canonical ABI or TAPI learns the new one. fetch-tapi-route.sh: routingPreference BEST_PRICE (CLASSIC is rejected), transcoding note. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The minHopPriceX36 ABI change is in v4-periphery MAIN (unreleased per-hop price limits), not in the UR unlock-fix branch — the branch is UR-main + one commit with the same periphery pin as UR-main. Any head-built router rejects TAPI's current v4 encoding until the next canonical UR release and TAPI roll out the new ABI together. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

… — drop the transcoder The Trading API encodes v4 actions per router ABI generation, selected with the public x-universal-router-version request header; the earlier incompatibility was the API's older default generation, not an ABI drift in this deployment. The fixture is refetched with 2.2.0 (matching this deployment's UR lineage; 2.1.1 verified byte-identical for v4 actions) and now replays verbatim — _transcodeTapiV4Actions deleted as promised. fetch-tapi-route.sh gains a UR_VERSION knob (default 2.2.0). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

… just the delta The UR SWEEP command is all-or-nothing, so the old delta gate produced an inconsistent policy: a pre-existing (donated/stranded) UR balance was captured whenever the route left a remainder but skipped when it didn't. Sweep on balance > 0 instead: UR balances are publicly sweepable, so any remainder is captured as the current caller's budget — consistent with the documented donation invariant — and the pre-call snapshot read is dropped. MockSwapRoute learns the SWEEP command so the reclaim leg is exercisable in unit tests; new differential test pins donated-native capture (the old code fails it: equal before/after balances skipped the sweep). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…ccuracy Three fuzzes for the math claims the existing suites do not discriminate: - trim inverse (pure, both branches): across the full sqrt-price/width domain with the debt correlated to the geometry (bounded by what a max-liquidity position holds over [lo,su]), burning the shipped round-up dl must free >= amountOut under v4's round-down burn. - fee-aware sizing accuracy: an add's dust must stay price-impact-small (<= 10bps) across fee tiers (0.5%..10%) and budget splits. This is the one test that fails if the fee discount is broken — every other suite tolerates the over-mint because the trim claws it back; only the dust betrays it. Verified discriminating: with the discount neutered in src it fails at 378x the bound on the first sample. - extreme-tick token1 mirror of the existing token0 full-deployment fuzz: >= 99.9% of feasible liquidity at any tick, exercising the price<1 numeraire branch of _sizeLiquidityWeighted. All held at 5000 runs. The uncorrelated-debt domain also surfaced a reachable-in-principle overflow revert in the token0 inverse (price within ~2^-64 of the upper boundary while the debt is still large -> mulDiv result exceeds uint256, EvmError instead of the lopt cap); atomic revert only, noted in the test comment, fix under discussion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Classified per the severity matrix: Low impact (atomic revert of the caller's own single attempt, transient, funds untouched) x Unlikely (requires extreme prices plus the post-swap price parked within sqrt-units of the range top while a large deficit remains — practically self-targeting only) = Informational. Surfaced by the full-domain trim fuzz in test/SwapAndAddMathFuzz.t.sol; documented at the math line rather than guarded, per the prove-reachability-or-document doctrine. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

_takeCredit(deficit) is load-bearing (the exact-input sell can overshoot a small remaining debt, leaving a tab credit that must reach the sweep). The surplus pair is defensive closure: step 2 pays the swap input in full and nothing re-touches that tab, so they are provably idle in every modeled flow — kept because they make the "both deltas are zero" post- condition locally checkable instead of a whole-contract argument. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…the debt When the deploy's CLOSE_CURRENCY credit (route-accrued fees on the advertised same-pool-route flow) retires the flash debt in reconcile step 1, the sell-all bought nothing the operation needed: it paid the pool fee converting the surplus into a refund and re-denominated it, contradicting the sweep doctrine. The guard hoists step 3's deficit- delta read and only swaps while a debt remains; the surplus now comes back in its own token. Pinned red-first by test_increase_creditRetiresDebt_skipsGratuitousSwap; the existing same-pool-route test's expectations updated to the corrected semantics (refund = credit minus flash debt, ~92% here, plus the token1 surplus). Also adds test/SwapAndAddFarEdge.t.sol — the referee for the claimed trim div-by-zero at the range's far edge: a deterministic tight-config grid plus a 5000-run fuzz hunting the state across the user-reachable space. Both hold: the sell's average execution is a geometric mean below spot while the fee-aware surplus is spot-sized net of fees, so full-range extraction is unaffordable and the price stays strictly inside the far edge on supported pools. That argument is now written at the trim INVARIANT, and the interface's unsupported-pools note gains the low-level-revert shape that returns-delta hooks (the only way to reach the edge) can produce. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

CI Test job: script/DeploySwapAndAdd.s.sol imports universal-router sources, whose own imports resolve through the submodule's yarn node_modules — present locally, absent on runners. The workflow now installs them (frozen lockfile, scripts ignored). CI Lint job: canonical formatting per the lint workflow's pinned foundry (v1.4.3, bare-mode `forge fmt --check`) — reflows one signature in SwapAndAdd.sol and the branch's PositionManager.t.sol touches. Note: the repo pins three different foundry versions (lint 1.4.3, test 1.3.6, dev machines newer) whose formatters disagree; formatting is canonicalized against the lint pin since that is the gate. Far-edge probe: extends the grid with tickSpacing-1 cells at 1..5-tick ranges and near-zero fee — the geometry where the quadratic impact margin is smallest and integer rounding is most of what separates the reconcile sell from the range edge — and adds a spacing dimension to the fuzz (widths down to a single tick). Held at 10000 runs: no raw reverts, price strictly inside the range on every success. The property-fuzz suite could not have caught this corner: its pools always carry a deep external band (other-LP output repays the debt before any edge) and exact-landing is a wei-precision event undirected runs do not sample. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The probe extension landed after the v1.4.3 formatting pass and was never re-formatted against the lint workflow's foundry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

zhongeric · 2026-08-25T17:27:40Z

@@ -0,0 +1,55 @@
+#!/usr/bin/env bash


should this be commited?

Should be fine. Its to generate real TAPI data for the fork e2e tests. The script is not used at runtime of the fork test, but allows to generate these hardcoded routes.

ccashwell

few small comments

ccashwell · 2026-08-25T19:52:55Z

+        // budget (native pool) or native route funding (non-native pool) — zero when neither applies.
+        uint256 value = address(this).balance;
+
+        universalRouter.execute{value: value}(commands, inputs);


because you're forwarding the entire native balance into the caller-supplied route here, any balance-relative UR command can overspend. for example, PAY_PORTION or SWEEP with a bps argument will size against the whole route's budget rather than just the swap output. suggest broadening the documented route construction constraint to note that all balance-relative commands (not just WRAP_ETH) are potential footguns.

This is for cases where you want to only swap parts of it because you use the other half for the pool right?
This lead me into looking at a broader inconsistency:
Multicall technically allows to batch operations (even though thats not the reason its implemented) for non native funds, but it fails for native funds. because all funds are swept to the recipient, while non native funds get pulled just in time for when they are actually needed. Difficult to handle this differently though, without increasing the risk of stranding funds, and an operator can still just batch call, instead of using the multicall to batch.
So added documentation to that additionally to your suggestion, see here:
8bd20a1

ccashwell · 2026-08-25T19:57:42Z

+            // KNOWN (informational): at extreme prices, with the post-swap price parked within sqrt-units of
+            // `sqrtUpper` while a large deficit remains, this quotient can exceed uint256 and revert (blank
+            // EvmError) before the `lopt` cap below would have answered — self-inflicted, atomic, transient.
+            dlUp = FullMath.mulDivRoundingUp(amountOut + 1, intermediate, sqrtUpper - lo);


you can fix this overflow edge case by comparing amountOut against getAmount0Delta(lo, sqrtUpper, lopt, false) first

I looked into it, the change seemed bigger then that to me? Definitely feel free to drop a PR or just add a commit if you see a way for this to be a small change with very limited area of affect

ccashwell · 2026-08-25T20:27:29Z

+    function compound(CompoundParams calldata params)
+        external
+        isNotLocked
+        checkDeadline(params.deadline)
+        returns (uint128 liquidityAdded, uint256 amount0, uint256 amount1)


not declared as payable but can consume a batch's msg.value under a multicall scenario. not exploitable afaict but confusing.

yeah, understand your issue with it... Do you see a simple solution though? Making compound payable just because of the multicall opportunity seems to hurt the interface more then help it

zhongeric · 2026-08-25T19:12:58Z

+        universalRouter.execute{value: value}(commands, inputs);
+        // Reclaim ANY native left in the UR: the operation's own over-push (the push is sized to the full
+        // held balance, so a remainder is expected), a route leg that produced native output on a non-native
+        // pool (a supported routeFunding flow that pushes zero value), or a pre-existing donation — UR
+        // balances are permissionlessly sweepable, so not a wei may be left there. The SWEEP takes the
+        // router's WHOLE balance; on a native pool it joins the caller's budget, otherwise it rests here
+        // until claimed (donation doctrine / a zero-amount address(0) routeFunding entry).
+        if (address(universalRouter).balance > 0) {
+            bytes[] memory sweepInputs = new bytes[](1);
+            // token ETH (address(0)), recipient MSG_SENDER (UR maps it back to this contract), no minimum.
+            sweepInputs[0] = abi.encode(address(0), ActionConstants.MSG_SENDER, 0);
+            universalRouter.execute(abi.encodePacked(UR_SWEEP_COMMAND), sweepInputs);
+        }


Could we just check that the last input / action is a sweep? Why make two calls?

It only runs if there is still balance inside the universal router, so its already limited to a route design that does not sweep as a last action.

zhongeric · 2026-08-25T19:15:19Z

+    /// @dev Size the position from the current holdings at the live price. Sizing is fee-aware: the side that
+    ///      will be swapped same-pool in `_reconcile` is discounted by that direction's total swap fee, so the
+    ///      optimistic deploy isn't over-sized by a fee the trim would otherwise claw back.
+    function _planLiquidity(CoreParams memory cp)


You only use a subset of the params in CoreParams here. Much cleaner to only take those, and consider writing a library for this logic to externalize it.

I moved everything that was actually pure math into its own library. It feels a bit strange to add functions with external calls to the library. If you feel strongly about it, let me know.

zhongeric · 2026-08-25T19:15:38Z

+        // the sizing math divides by the price; only initialized pools guarantee sp >= MIN_SQRT_PRICE.
+        if (sqrtPriceX96 == 0) revert IPoolManager.PoolNotInitialized();


Feel like this check should be moved outside of this helper func

zhongeric · 2026-08-25T19:16:05Z

+    ///      inside the token transfer — the same state leaves the reconcile swap nothing to source the deficit
+    ///      from, so the operation is unviable regardless and no pre-check is spent on it (the deficit token is
+    ///      usually a cold SLOAD, i.e. real gas on every call to buy a prettier error in an already-doomed state).
+    function _flashTakeDeficit(CoreParams memory cp, uint256 amount0, uint256 amount1) internal {


no need for an internal func here IMO

generally agree, but even after reducing the natspec, this function still does relevant documentation work. Purely because of that aspect, I lean towards leaving as is

zhongeric · 2026-08-25T19:35:07Z

+    struct RebalanceParams {
+        uint256 tokenId;
+        int128 additional0;
+        int128 additional1;
+        int24 newTickLower;
+        int24 newTickUpper;
+        bytes route;
+        TokenAmount[] routeFunding;
+        uint256 minLiquidity;
+        address recipient;
+        bytes hookData;
+        uint256 deadline;
+    }


I'd like to see more composition between the struct types here. I feel like we have Position entities duplicated / flattened / renamed across the diff structs.

- 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 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

zhongeric

Can we add unit tests for the many internal functions? Namely, deployLiquidity and reconcile. These functions have lots of biz logic and conditional branching that isn't really tested.

zhongeric · 2026-08-26T18:29:42Z

+                FullMath.mulDiv(FixedPoint96.Q96, FixedPoint96.Q96, sqrtPriceX96), FixedPoint96.Q96, sqrtPriceX96
+            );
+
+        uint256 refValue;


nit: weightedRefLiquidity

zhongeric · 2026-08-26T18:30:56Z

+    }
+
+    /// @dev Values a token pair in the cheaper-token numeraire, weighting each side by its pips factor.
+    function _weightedValue(


So this actually does more than just apply a weight, which is unclear from the function name. Would be nice to break this up and/or have the calling function do the conditional logic

zhongeric · 2026-08-26T18:59:57Z

+
+    /// @dev Executes a same-pool swap to convert surplus tokens to deficit without price limits.
+    ///      Unbounded because `minLiquidity` is the slippage gate and input amount is strictly bounded by held holdings.
+    function _swap(PoolKey memory key, bool zeroForOne, int256 amountSpecified, bytes memory hookData) internal {


Also don't really see the benefit of having htis be its own function - if anything, it's misleading because it applies defaults to the swap that affect its behavior with no indication

zhongeric · 2026-08-26T19:00:48Z

+        // Validate msg.value: exactly one native contribution (pool budget OR route funding) is allowed.
+        if (msg.value != expectedValue) revert InvalidEthValue();
+        // In multicall batches, verify balance was not already spent by an earlier subcall.
+        if (address(this).balance < expectedValue) revert InvalidEthValue();


combine with the prev if statement with ||?

zhongeric · 2026-08-26T19:05:46Z

+        // Reclaim any unspent native ETH left in Universal Router (UR balances are permissionlessly sweepable).
+        if (address(universalRouter).balance > 0) {
+            bytes[] memory sweepInputs = new bytes[](1);
+            sweepInputs[0] = abi.encode(address(0), ActionConstants.MSG_SENDER, 0);
+            universalRouter.execute(abi.encodePacked(UR_SWEEP_COMMAND), sweepInputs);
+        }


Hm still not convinced we need this. Why can't we just rely on the caller to do this?

zhongeric · 2026-08-26T19:06:36Z

+            )
+            : abi.encodePacked(deployAction, uint8(Actions.CLOSE_CURRENCY), uint8(Actions.CLOSE_CURRENCY));
+
+        bytes[] memory params = new bytes[](isNative ? 4 : 3);


quite a bit cleaner to just build this in assembly and overwrite the length pointer imo

zhongeric · 2026-08-26T19:07:10Z

+        // Native pools carry a trailing SWEEP to return unconsumed wei of the forwarded ETH.
+        Currency c0 = cp.key.currency0;
+        bool isNative = c0.isAddressZero();
+        bytes memory actions = isNative


generally want to try to reduce the number of conditional branches and linearize as much as possible

zhongeric · 2026-08-26T19:13:54Z

+        bytes[] memory params = new bytes[](2);
+        params[0] = abi.encode(tokenId, uint128(0), uint128(0), hookData);
+        params[1] = abi.encode(key.currency0, key.currency1, ActionConstants.MSG_SENDER);
+        positionManager.modifyLiquidities(abi.encode(actions, params), type(uint256).max);


consider making the deadline block.timestamp since it doesn't matter either

Trim the AI-generated NatSpec and inline comments across SwapAndAdd, ISwapAndAdd, SwapAndAddMath, the deploy script, and config comments. Comments now explain why rather than what, in short plain sentences. Addresses review feedback on PR #591: - SwapAndAddMath: clearer REFERENCE_LIQUIDITY and cheaper-token numeraire comments (cheaper or equal), and the liquidity scaling now reads budgetValue * REFERENCE_LIQUIDITY / refValue (mulDiv is commutative, no behavior change) - SwapAndAdd._swap: natspec now states max slippage is safe because callers enforce minLiquidity, and that callers MUST check minimums Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…ts (#594) * docs: rewrite src and script documentation in plain, terse style Trim the AI-generated NatSpec and inline comments across SwapAndAdd, ISwapAndAdd, SwapAndAddMath, the deploy script, and config comments. Comments now explain why rather than what, in short plain sentences. Addresses review feedback on PR #591: - SwapAndAddMath: clearer REFERENCE_LIQUIDITY and cheaper-token numeraire comments (cheaper or equal), and the liquidity scaling now reads budgetValue * REFERENCE_LIQUIDITY / refValue (mulDiv is commutative, no behavior change) - SwapAndAdd._swap: natspec now states max slippage is safe because callers enforce minLiquidity, and that callers MUST check minimums Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(test): greatly simplify test suite comments Comments now state what each test does and the intended effect, one or two lines each. Bug-history narratives, premise essays, and restated mechanics are removed. Fork blocks, TAPI route provenance, and magic-number meanings are kept as one-liners. Comment-only change: all 21 files verified byte-identical to the previous commit after stripping comments. 163 unit tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: small clarifications * docs: invariant correction --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: mgretzke <m.gretzke@vimage.de>

mgretzke and others added 30 commits August 25, 2026 19:30

chore: drop stale SwapAndAdd design doc

dad06d2

The shipped design diverged (mint-to-contract + trim and route-first were out of scope there); the normative docs now live in ISwapAndAdd's NatSpec and audits/SwapAndAdd-audit-scope.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

style(test): forge fmt whitespace in test helpers

b143efd

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

mgretzke and others added 5 commits August 25, 2026 19:30

docs(tests): replace internal review shorthand with plain language

006ad4b

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

mgretzke force-pushed the feat/swap-and-add branch from 4a073aa to 006ad4b Compare August 25, 2026 17:33

mgretzke and others added 2 commits August 25, 2026 20:50

style: format the extended far-edge probe with the lint pin

c56e484

The probe extension landed after the v1.4.3 formatting pass and was never re-formatted against the lint workflow's foundry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

zhongeric reviewed Aug 25, 2026

View reviewed changes

ccashwell reviewed Aug 25, 2026

View reviewed changes

zhongeric reviewed Aug 26, 2026

View reviewed changes

mgretzke and others added 6 commits August 26, 2026 11:13

docs: fully reworked natspec

7752d9a

refactor: moved pure math into library

3d10f77

fix: pinned pragma on deployable contract

5496f92

fix: reuse calculated sqrtLower and sqrtUpper

cb2b69e

fix: renaming internal functions

0da496e

mgretzke force-pushed the feat/swap-and-add branch 2 times, most recently from 5abe322 to f437b79 Compare August 26, 2026 16:32

refactor: improved readability of _deployLiquidity

49dfaa3

zhongeric reviewed Aug 26, 2026

View reviewed changes

mgretzke added 3 commits August 27, 2026 12:23

fix: zero liquidity increases and gas snapshots

18338b8

docs: improved documentation

8bd20a1

docs: trustmodel updated

bc6d8d6

zhongeric mentioned this pull request Aug 27, 2026

docs: trim SwapAndAdd documentation to minimal, plain-language comments #594

Merged

mgretzke changed the title ~~feat: SwapAndAdd — route-first swap-and-add zap for v4 (add / rebalance / increase / compound)~~ feat: SwapAndAdd — (add / rebalance / increase / compound) Aug 27, 2026

refactor: Renaming and small restructure

3784628

		// the sizing math divides by the price; only initialized pools guarantee sp >= MIN_SQRT_PRICE.
		if (sqrtPriceX96 == 0) revert IPoolManager.PoolNotInitialized();

Uh oh!

Conversation

mgretzke commented Aug 25, 2026

Summary

Design

Known limits (documented in ISwapAndAdd)

Testing

Deployment note

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ccashwell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zhongeric left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Known limits (documented in `ISwapAndAdd`)