diff --git a/skills/deploy-uni-hook/SKILL.md b/skills/deploy-uni-hook/SKILL.md index 0b326daa73b..363d7392ae5 100644 --- a/skills/deploy-uni-hook/SKILL.md +++ b/skills/deploy-uni-hook/SKILL.md @@ -93,6 +93,7 @@ These are standing defects measured on the live fleet. Freeform MUST NOT recreat - 1c. Shared counter an attacker can park, not advanced on failure: griefing primitive. - 2. A contract in the `unlock` frame can satisfy the predicate; a signed tx cannot. That binds the wrong party. - 5. Never compare raw `amountSpecified` to a token-denominated constant. The caller picks the specified currency via exact-in vs exact-out. Use a dimensionless bound (tick move / liquidity fraction). +- 6. A "balance" / "skew" / "heavier-side" gate on the two virtual reserves is a raw-PRICE gate in disguise. `StateLibrary` gives `amount0 = L*2^96/sqrtP` and `amount1 = L*sqrtP/2^96`, so `amount0/amount1 = 1/price` and the liquidity `L` cancels exactly. Any test on the two reserves (`b0 >= b1`, a skew band, "which side is heavier") therefore reduces to comparing the pool's RAW price to an implicit `1.0`, and raw price is `token1/token0` in smallest units, so it is ~1.0 only for a same-decimals pair near parity. A USDC(6d)/WETH(18d) pool sits ~8 orders of magnitude off; two 18-dec tokens at price 2.0 are already outside a 10% band. Such a gate is permanently one-directional on every real pair: one whole leg reverts forever, and "every trade rebalances toward 50/50" is false (a full-range position is already 50/50 by value at any price). Fix: snapshot the pool's OWN reference (its `sqrtPriceX96` at `afterInitialize`, or an explicit target ratio from the brief) and gate the current price against THAT, never a hard-coded 1.0. This needs the `afterInitialize` callback (adds flag bit `0x1000`), so a price/balance/skew hook must include it in the callback set or it cannot know its own starting price. - Never `balanceOf(poolManager)`: that is the v4 singleton's global inventory, not this pool. Use `StateLibrary`. - `sender` is the router. Do not treat it as the trader. @@ -100,6 +101,7 @@ These are standing defects measured on the live fleet. Freeform MUST NOT recreat - A fee hook must assert the take on exact-in AND exact-out. - A gate needs a hookless negative control (`hooks = address(0)`). - Do not cache `block.number` across `vm.roll` (via-ir folds it). Use `vm.getBlockNumber()`. +- A price / balance / skew gate MUST be asserted at a price away from 1:1. The scaffold's `setUp()` pool starts at 1:1 (`sqrtPriceX96 = 2^96`), the single price where a raw-price-vs-1.0 gate looks correct no matter how it is written. Call `_freshPoolAt()` (helper in `Hook.t.sol`) and assert BOTH legs there: the leg that must stay open is not rejected, the leg that must close reverts. A gate proven only at 1:1 is a false pass. @@ -113,8 +115,8 @@ These are standing defects measured on the live fleet. Freeform MUST NOT recreat 4. **Build the hook (brief-driven).** - **Template mode** (`dynamic` / `noop` / `skim`): in `$HOOKBUILD_DIR/src/.sol`, edit ONLY the region between `// --- AEON:LOGIC START ---` and `// --- AEON:LOGIC END ---`. Keep the callback signatures and flag set unchanged. If the default already fits the brief, leave it. - - **Freeform mode** (anything else): write the whole hook into `$HOOKBUILD_DIR/src/Hook.sol` — replace the `// --- AEON:BODY ... ---` region. Rules: keep the contract name `Hook` and `constructor(IPoolManager)`; implement whichever v4 callbacks the prompt needs, each with the EXACT `IHooks` signature, `onlyPoolManager`, and the right selector return. Do NOT hand-set flags — they are auto-derived from which callbacks you implement. If a callback returns a non-zero delta, set `HOOK_RETURNS_DELTA` in `$HOOKBUILD_DIR/hook.env`; for a fee-override hook set `HOOK_POOL_FEE=dynamic` there. Follow **Labs routing** and **Fleet audit rules** above: empty `hookData` must succeed; a game must not revert a vanilla exact-in swap; a `take()` must declare `HOOK_RETURNS_DELTA`, charge magnitude (exact-in and exact-out), and never custody. - - **Also write the behavioral test.** In `$HOOKBUILD_DIR/test/Hook.t.sol`, replace the `// --- AEON:ASSERT ... ---` region with `test_*` functions that assert the hook's SPECIFIC intended behavior — not just "does not revert". For every rule in the brief write at least one positive and one negative case: a swap the hook must REJECT as `_expectSwapRevert(zeroForOne, amount, Hook.SomeError.selector)` (this helper unwraps v4's `WrappedError` for you — do NOT use bare `vm.expectRevert`, it won't match the wrapper); a swap it must ALLOW as a plain `_swap(...)`; any getter/accounting as `assertEq(hook.someGetter(...), expected)`. Do NOT edit `setUp()` or the helpers — only the `AEON:ASSERT` region. If the brief has no rejectable behavior, still assert the observable state the hook changes. + - **Freeform mode** (anything else): write the whole hook into `$HOOKBUILD_DIR/src/Hook.sol` — replace the `// --- AEON:BODY ... ---` region. Rules: keep the contract name `Hook` and `constructor(IPoolManager)`; implement whichever v4 callbacks the prompt needs, each with the EXACT `IHooks` signature, `onlyPoolManager`, and the right selector return. Do NOT hand-set flags — they are auto-derived from which callbacks you implement. If a callback returns a non-zero delta, set `HOOK_RETURNS_DELTA` in `$HOOKBUILD_DIR/hook.env`; for a fee-override hook set `HOOK_POOL_FEE=dynamic` there. Follow **Labs routing** and **Fleet audit rules** above: empty `hookData` must succeed; a game must not revert a vanilla exact-in swap; a `take()` must declare `HOOK_RETURNS_DELTA`, charge magnitude (exact-in and exact-out), and never custody; a price/balance/skew gate must add the `afterInitialize` callback and anchor to the pool's own start price (Gates rule 6), never an implicit 1.0. + - **Also write the behavioral test.** In `$HOOKBUILD_DIR/test/Hook.t.sol`, replace the `// --- AEON:ASSERT ... ---` region with `test_*` functions that assert the hook's SPECIFIC intended behavior — not just "does not revert". For every rule in the brief write at least one positive and one negative case: a swap the hook must REJECT as `_expectSwapRevert(zeroForOne, amount, Hook.SomeError.selector)` (this helper unwraps v4's `WrappedError` for you — do NOT use bare `vm.expectRevert`, it won't match the wrapper); a swap it must ALLOW as a plain `_swap(...)`; any getter/accounting as `assertEq(hook.someGetter(...), expected)`. For a gate whose decision depends on price or reserve balance, assert it through `_freshPoolAt()` (both legs, off parity) - `setUp()`'s pool is at 1:1, where such a gate always looks correct. Do NOT edit `setUp()` or the helpers - only the `AEON:ASSERT` region. If the brief has no rejectable behavior, still assert the observable state the hook changes. 5. **Simulate + audit (always).** Pass mode, kind, and chain (chain omitted = `base-sepolia`): ```bash diff --git a/skills/deploy-uni-hook/templates/Hook.t.sol b/skills/deploy-uni-hook/templates/Hook.t.sol index f5364907121..54403f61848 100644 --- a/skills/deploy-uni-hook/templates/Hook.t.sol +++ b/skills/deploy-uni-hook/templates/Hook.t.sol @@ -11,8 +11,12 @@ pragma solidity 0.8.26; // prompt asked, not just that it does not revert. // // setUp() below is fixed scaffolding: it forks the target chain, mines + deploys the -// generated Hook with its auto-derived flags, opens a fresh pool, and adds deep +// generated Hook with its auto-derived flags, opens a fresh pool AT 1:1, and adds deep // liquidity. The generator MUST NOT edit setUp() or the helpers — only AEON:ASSERT. +// For any gate whose decision depends on price or reserve balance, DO NOT rely on the +// 1:1 pool alone: call _freshPoolAt() inside a test to repoint +// the pool off parity, then assert both legs. A price/balance/skew gate looks correct +// at 1:1 no matter how it is written, so testing only there proves nothing. import {Test} from "forge-std/Test.sol"; @@ -43,12 +47,13 @@ contract HookBehaviorTest is Test { PoolSwapTest internal swapRouter; MockERC20 internal tA; MockERC20 internal tB; + uint24 internal poolFee; function setUp() public { pm = IPoolManager(vm.envAddress("POOL_MANAGER")); uint160 flags = uint160(vm.envUint("HOOK_FLAGS")); string memory pf = vm.envOr("HOOK_POOL_FEE", string("3000")); - uint24 poolFee = + poolFee = keccak256(bytes(pf)) == keccak256("dynamic") ? LPFeeLibrary.DYNAMIC_FEE_FLAG : uint24(vm.parseUint(pf)); // tokens @@ -134,6 +139,44 @@ contract HookBehaviorTest is Test { return false; } + /// @dev Repoint the test pool to a SECOND pool on the same hook, started at an + /// arbitrary price, so `_swap` / `_expectSwapRevert` and any reserve/skew getter + /// run OFF parity. Fresh tokens give it a distinct pool id (reusing the same + /// tokens+fee+hook would collide with setUp's 1:1 pool). Initializing the new pool + /// also fires the hook's afterInitialize on it, so a correctly price-anchored gate + /// snapshots THIS pool's price as its reference. Any gate whose decision depends on + /// price or reserve balance MUST be asserted through this helper, not only at 1:1. + /// Pick a MODERATE off-parity price (e.g. 4.0) — the bug surfaces at any price + /// outside the band, and extreme prices can exceed the minted token headroom. + /// price 2.0 -> 112045541949572279837463876096 + /// price 4.0 -> 158456325028528675187087900672 + /// price 0.25 -> 39614081257132168796771975168 + function _freshPoolAt(uint160 sqrtPriceX96) internal { + tA = new MockERC20("Hook Test A2", "HTA2"); + tB = new MockERC20("Hook Test B2", "HTB2"); + tA.mint(address(this), 1e33); + tB.mint(address(this), 1e33); + (Currency c0, Currency c1) = address(tA) < address(tB) + ? (Currency.wrap(address(tA)), Currency.wrap(address(tB))) + : (Currency.wrap(address(tB)), Currency.wrap(address(tA))); + key = PoolKey({currency0: c0, currency1: c1, fee: poolFee, tickSpacing: 60, hooks: IHooks(address(hook))}); + pm.initialize(key, sqrtPriceX96); + tA.approve(address(lpRouter), type(uint256).max); + tB.approve(address(lpRouter), type(uint256).max); + tA.approve(address(swapRouter), type(uint256).max); + tB.approve(address(swapRouter), type(uint256).max); + lpRouter.modifyLiquidity( + key, + IPoolManager.ModifyLiquidityParams({ + tickLower: TickMath.minUsableTick(60), + tickUpper: TickMath.maxUsableTick(60), + liquidityDelta: 1e23, + salt: bytes32(0) + }), + "" + ); + } + // --- AEON:ASSERT START (freeform: replace with behavioral tests for the prompt) --- // Default scaffold matches the default Hook body (an afterSwap swap counter). // The generator MUST replace this with assertions specific to the generated hook: @@ -142,6 +185,9 @@ contract HookBehaviorTest is Test { // CustomRevert.WrappedError, so vm.expectRevert(selector) never matches) // - a swap the hook is meant to ALLOW -> _swap(...) with no revert // - any getter / accounting -> assertEq(hook.someGetter(...), expected) + // - a PRICE / BALANCE / SKEW gate -> also assert it through _freshPoolAt(): + // the leg that must stay open is NOT rejected and the leg that must close reverts, + // at a price away from parity. Only-at-1:1 is a false pass (see the header note). function test_defaultCounterIncrements() public { _swap(true, -1e15); assertEq(hook.swapCount(key.toId()), 1, "counter did not increment");