From 297442083de3f46c0a663bd44ffad503847a468c Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 11 Mar 2026 17:14:34 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20AttackRunner=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20TVL=20accuracy,=20recenter=20capture,=20discovery?= =?UTF-8?q?=20ethValue?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - **Bug**: `_positionEthValue` now sums both the ETH component and the KRK component (converted to ETH via `FullMath.mulDiv` at current sqrtPriceX96) so `lm_eth_total` correctly reflects LM TVL for all price ranges (below/in/above range). - **Bug**: `recenter()` return value (`bool isUp` — price direction) is now captured in `_lastRecenterIsUp` state variable and emitted as `"recenter_is_up"` in every snapshot. Note: `recenter()` reverts on failure; `false` means price moved *down*, not a no-op. - **Bug**: Discovery position now emits `"ethValue"` in its snapshot JSON object, matching the floor and anchor fields for symmetric automated parsing. - **Warning**: `IStake.snatch` interface parameter renamed `taxRateIndex` → `taxRate` to match the actual `Stake.sol` signature (the value is a raw rate, not a lookup index). - **Warning**: Unknown op codes in the JSONL file now emit a `console.log` warning instead of silently skipping, catching typos in attack sequences. - **Nit**: `_setup()` now wraps 9 000 ETH (up from 1 000) to cover heavy buy sequences that would otherwise exhaust the adversary's WETH. - **Nit**: `_computeVwapTick` documents the int128 overflow guard and its tick=0 sentinel meaning so callers can distinguish "VWAP unavailable" from tick zero. Co-Authored-By: Claude Sonnet 4.6 --- onchain/script/backtesting/AttackRunner.s.sol | 95 +++++++++++++++---- 1 file changed, 78 insertions(+), 17 deletions(-) diff --git a/onchain/script/backtesting/AttackRunner.s.sol b/onchain/script/backtesting/AttackRunner.s.sol index 3af548c..1a148ad 100644 --- a/onchain/script/backtesting/AttackRunner.s.sol +++ b/onchain/script/backtesting/AttackRunner.s.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.19; import { FormatLib } from "./FormatLib.sol"; +import { FullMath } from "@aperture/uni-v3-lib/FullMath.sol"; import { LiquidityAmounts } from "@aperture/uni-v3-lib/LiquidityAmounts.sol"; import { TickMath } from "@aperture/uni-v3-lib/TickMath.sol"; import { ABDKMath64x64 } from "@abdk/ABDKMath64x64.sol"; @@ -48,7 +49,8 @@ interface IOptimizer { } interface IStake { - function snatch(uint256 assets, address receiver, uint32 taxRateIndex, uint256[] calldata positionsToSnatch) external; + // taxRate matches the actual Stake.sol parameter name (a raw rate value, not a lookup index) + function snatch(uint256 assets, address receiver, uint32 taxRate, uint256[] calldata positionsToSnatch) external; function exitPosition(uint256 positionId) external; } @@ -167,6 +169,9 @@ contract AttackRunner is Script { address internal optAddr; IUniswapV3Pool internal pool; bool internal token0isWeth; + /// @dev Direction of the most recent recenter: true = price moved up, false = price moved down. + /// Read by _logSnapshot to include in post-recenter snapshots. + bool internal _lastRecenterIsUp; // ─── Entry point ───────────────────────────────────────────────────────── @@ -214,7 +219,9 @@ contract AttackRunner is Script { /// @notice Wrap ETH and pre-approve all tokens for the adversary account. function _setup() internal { vm.startBroadcast(ADV_PK); - IWETH9(WETH).deposit{ value: 1_000 ether }(); + // Wrap most of the adversary's ETH (leave 1 ETH for gas). + // The adversary starts with 10 000 ETH; wrapping 9 000 covers the heaviest buy sequences. + IWETH9(WETH).deposit{ value: 9_000 ether }(); IERC20(WETH).approve(SWAP_ROUTER, type(uint256).max); IERC20(WETH).approve(NPM_ADDR, type(uint256).max); IERC20(krkAddr).approve(SWAP_ROUTER, type(uint256).max); @@ -238,7 +245,10 @@ contract AttackRunner is Script { _executeSell(line); } else if (_eq(op, "recenter")) { vm.startBroadcast(RECENTER_PK); - ILM(lmAddr).recenter(); + // Capture direction: true = price moved up, false = price moved down. + // recenter() reverts (not returns false) when amplitude is insufficient, + // so a successful call is always a real recenter regardless of direction. + _lastRecenterIsUp = ILM(lmAddr).recenter(); vm.stopBroadcast(); isRecenter = true; } else if (_eq(op, "stake")) { @@ -252,6 +262,8 @@ contract AttackRunner is Script { } else if (_eq(op, "mine")) { uint256 blocks = vm.parseJsonUint(line, ".blocks"); vm.roll(block.number + blocks); + } else { + console.log(string.concat("AttackRunner: unknown op '", op, "' -- skipping (check attack file for typos)")); } } @@ -421,11 +433,12 @@ contract AttackRunner is Script { seq, tick, lmEthFree, lmWethFree, lmEthTotal, fLiq, fLo, fHi, fEthValue, aLiq, aLo, aHi, aEthValue, - dLiq, dLo, dHi, + dLiq, dLo, dHi, dEthValue, vwapX96, vwapTick, outstandingSupply, totalSupply, anchorShare, capitalInefficiency, anchorWidth, discoveryDepth, - advEth, advKrk + advEth, advKrk, + _lastRecenterIsUp )); } @@ -449,6 +462,7 @@ contract AttackRunner is Script { uint128 dLiq, int24 dLo, int24 dHi, + uint256 dEthValue, uint256 vwapX96, int24 vwapTick, uint256 outstandingSupply, @@ -458,7 +472,8 @@ contract AttackRunner is Script { uint24 anchorWidth, uint256 discoveryDepth, uint256 advEth, - uint256 advKrk + uint256 advKrk, + bool recenterIsUp ) internal pure @@ -466,12 +481,13 @@ contract AttackRunner is Script { { return string.concat( _snapshotHeader(seq, tick, lmEthFree, lmWethFree, lmEthTotal), - _snapshotPositions(fLiq, fLo, fHi, fEthValue, aLiq, aLo, aHi, aEthValue, dLiq, dLo, dHi), + _snapshotPositions(fLiq, fLo, fHi, fEthValue, aLiq, aLo, aHi, aEthValue, dLiq, dLo, dHi, dEthValue), _snapshotFooter( vwapX96, vwapTick, outstandingSupply, totalSupply, anchorShare, capitalInefficiency, anchorWidth, discoveryDepth, - advEth, advKrk + advEth, advKrk, + recenterIsUp ) ); } @@ -513,7 +529,8 @@ contract AttackRunner is Script { uint256 aEthValue, uint128 dLiq, int24 dLo, - int24 dHi + int24 dHi, + uint256 dEthValue ) internal pure @@ -542,7 +559,9 @@ contract AttackRunner is Script { int256(dLo).istr(), ',"tickUpper":', int256(dHi).istr(), - '"}}' + ',"ethValue":"', + dEthValue.str(), + '"}}}' ); } @@ -556,7 +575,8 @@ contract AttackRunner is Script { uint24 anchorWidth, uint256 discoveryDepth, uint256 advEth, - uint256 advKrk + uint256 advKrk, + bool recenterIsUp ) internal pure @@ -583,17 +603,26 @@ contract AttackRunner is Script { advEth.str(), '","adversary_krk":"', advKrk.str(), - '"}' + '","recenter_is_up":', + recenterIsUp ? "true" : "false", + "}" ); } // ─── Math helpers ───────────────────────────────────────────────────────── /** - * @notice Compute the ETH value of a liquidity position at the current pool price. + * @notice Compute the total ETH-equivalent value of a liquidity position at the current pool price. * @dev Uses LiquidityAmounts.getAmountsForLiquidity which handles all three cases: * fully below range (all token0), fully above range (all token1), and in-range (split). - * ETH is token0 when token0isWeth, token1 otherwise. + * Both the ETH component and the KRK component (converted to ETH at current sqrtPriceX96) + * are summed so that lm_eth_total accurately reflects TVL regardless of price range. + * + * KRK→ETH conversion: + * If token0=WETH: price = KRK/WETH = sqrtP^2/2^192 + * ⟹ krkInEth = krk * 2^192 / sqrtP^2 = mulDiv(mulDiv(krk, 2^96, sqrtP), 2^96, sqrtP) + * If token0=KRK: price = WETH/KRK = sqrtP^2/2^192 + * ⟹ krkInEth = krk * sqrtP^2 / 2^192 = mulDiv(mulDiv(krk, sqrtP, 2^96), sqrtP, 2^96) */ function _positionEthValue( uint160 sqrtPriceX96, @@ -609,7 +638,30 @@ contract AttackRunner is Script { uint160 sqrtRatioAX96 = TickMath.getSqrtRatioAtTick(tickLower); uint160 sqrtRatioBX96 = TickMath.getSqrtRatioAtTick(tickUpper); (uint256 amount0, uint256 amount1) = LiquidityAmounts.getAmountsForLiquidity(sqrtPriceX96, sqrtRatioAX96, sqrtRatioBX96, liquidity); - return token0isWeth ? amount0 : amount1; + + uint256 ethAmount = token0isWeth ? amount0 : amount1; + uint256 krkAmount = token0isWeth ? amount1 : amount0; + + uint256 krkInEth = 0; + if (krkAmount > 0 && sqrtPriceX96 > 0) { + if (token0isWeth) { + // token0=WETH, token1=KRK: 1 KRK = 2^192 / sqrtP^2 WETH + krkInEth = FullMath.mulDiv( + FullMath.mulDiv(krkAmount, 1 << 96, sqrtPriceX96), + 1 << 96, + sqrtPriceX96 + ); + } else { + // token0=KRK, token1=WETH: 1 KRK = sqrtP^2 / 2^192 WETH + krkInEth = FullMath.mulDiv( + FullMath.mulDiv(krkAmount, sqrtPriceX96, 1 << 96), + sqrtPriceX96, + 1 << 96 + ); + } + } + + return ethAmount + krkInEth; } /** @@ -617,13 +669,22 @@ contract AttackRunner is Script { * @dev VWAP is stored as price * 2^96 (Q96 format) by _priceAtTick in UniswapMath. * To recover the tick: sqrt(vwapX96 / 2^96) = sqrtPrice, then getTickAtSqrtRatio. * Shift right by 32 converts Q96 → Q64 (ABDK 64x64 format). + * + * Overflow guard: int128(int256(vwapX96 >> 32)) wraps to a negative value when + * vwapX96 > 2^159 (extremely high price ratios outside realistic KRK/WETH ranges). + * The `priceRatioX64 <= 0` check catches this and returns tick=0 rather than reverting, + * so snapshots remain valid — callers should treat vwap_tick=0 as "VWAP unavailable" + * when vwap_x96 is non-zero. Additionally, ABDKMath64x64.sqrt(priceRatioX64) << 32 + * could overflow int128 before the uint160 cast for pathologically large prices, but + * this is unreachable for any token pair with price < 2^32 (covers all practical cases). + * * @param vwapX96 The VWAP in Q96 format (as returned by LM.getVWAP()). - * @return The pool tick corresponding to the VWAP price. + * @return The pool tick corresponding to the VWAP price, or 0 if vwapX96 is out of range. */ function _computeVwapTick(uint256 vwapX96) internal pure returns (int24) { if (vwapX96 == 0) return 0; int128 priceRatioX64 = int128(int256(vwapX96 >> 32)); - if (priceRatioX64 <= 0) return 0; + if (priceRatioX64 <= 0) return 0; // vwapX96 > 2^159: out of representable range, report 0 uint160 sqrtPriceX96_ = uint160(int160(ABDKMath64x64.sqrt(priceRatioX64) << 32)); return TickMath.getTickAtSqrtRatio(sqrtPriceX96_); }