From 53c14745cb61ab63fca8794c8c47b142202898d0 Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 11 Mar 2026 08:17:49 +0000 Subject: [PATCH 1/3] fix: Replace anchor-midpoint VWAP with pool.observe() TWAP oracle (#575) - Add lastRecenterTimestamp to track recenter interval for TWAP - Increase pool observation cardinality to 100 in constructor - In _scrapePositions, use pool.observe([elapsed, 0]) to get TWAP tick over the full interval between recenters; falls back to anchor midpoint when elapsed==0 or pool.observe() reverts (insufficient history) - Add test_twapReflectsAveragePriceNotJustLastSwap: verifies TWAP-based VWAP reflects the average price across the recenter interval, not just the last-swap anchor snapshot Co-Authored-By: Claude Sonnet 4.6 --- onchain/src/LiquidityManager.sol | 49 ++++++++++-- onchain/src/VWAPTracker.sol | 6 +- onchain/test/VWAPFloorProtection.t.sol | 100 +++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 6 deletions(-) diff --git a/onchain/src/LiquidityManager.sol b/onchain/src/LiquidityManager.sol index b6d24eb..617ed78 100644 --- a/onchain/src/LiquidityManager.sol +++ b/onchain/src/LiquidityManager.sol @@ -56,6 +56,9 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { /// @notice Minimum seconds between open recenters (when recenterAccess is unset) uint256 internal constant MIN_RECENTER_INTERVAL = 60; + /// @notice Last recenter block timestamp — used to compute elapsed interval for pool TWAP oracle + uint256 public lastRecenterTimestamp; + /// @notice Emitted on each successful recenter for monitoring and indexing event Recentered(int24 indexed currentTick, bool indexed isUp); @@ -83,6 +86,9 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { kraiken = Kraiken(_kraiken); token0isWeth = _WETH9 < _kraiken; optimizer = Optimizer(_optimizer); + // Increase observation cardinality so pool.observe() has sufficient history + // for TWAP calculations between recenters. + IUniswapV3Pool(PoolAddress.computeAddress(factory, poolKey)).increaseObservationCardinalityNext(100); } /// @notice Callback function for Uniswap V3 mint operations @@ -141,7 +147,9 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { require(block.timestamp >= lastRecenterTime + MIN_RECENTER_INTERVAL, "recenter cooldown"); require(_isPriceStable(currentTick), "price deviated from oracle"); } + uint256 prevTimestamp = lastRecenterTimestamp; lastRecenterTime = block.timestamp; + lastRecenterTimestamp = block.timestamp; // Check if price movement is sufficient for recentering isUp = false; @@ -177,7 +185,7 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { } lastRecenterTick = currentTick; - _scrapePositions(shouldRecordVWAP); + _scrapePositions(shouldRecordVWAP, prevTimestamp); // Update total supply tracking if price moved up if (isUp) { @@ -212,7 +220,8 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { /// @notice Removes all positions and collects fees /// @param recordVWAP Whether to record VWAP (only when net ETH outflow / price fell since last recenter, or at bootstrap) - function _scrapePositions(bool recordVWAP) internal { + /// @param prevTimestamp The block.timestamp of the previous recenter, used to compute TWAP interval + function _scrapePositions(bool recordVWAP, uint256 prevTimestamp) internal { uint256 fee0 = 0; uint256 fee1 = 0; uint256 currentPrice; @@ -230,10 +239,10 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { fee0 += collected0 - amount0; fee1 += collected1 - amount1; - // Record price from anchor position for VWAP + // Record price from anchor position for VWAP using pool TWAP oracle. + // Falls back to anchor midpoint when elapsed == 0 or pool.observe() reverts. if (i == uint256(Stage.ANCHOR)) { - int24 tick = position.tickLower + ((position.tickUpper - position.tickLower) / 2); - currentPrice = _priceAtTick(token0isWeth ? -1 * tick : tick); + currentPrice = _getTWAPOrFallback(prevTimestamp, position.tickLower, position.tickUpper); } } } @@ -265,6 +274,36 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { } } + /// @notice Computes price using pool TWAP oracle between prevTimestamp and now. + /// @dev Falls back to anchor midpoint when the interval is zero or pool.observe() reverts + /// (e.g. insufficient observation history on first recenter or very short intervals). + /// @param prevTimestamp Timestamp of the previous recenter (0 on first recenter) + /// @param tickLower Lower tick of the anchor position (used for fallback midpoint) + /// @param tickUpper Upper tick of the anchor position (used for fallback midpoint) + /// @return priceX96 Price in Q96 format (price * 2^96) + function _getTWAPOrFallback(uint256 prevTimestamp, int24 tickLower, int24 tickUpper) + internal + view + returns (uint256 priceX96) + { + // Only attempt TWAP when there is a measurable elapsed interval + if (prevTimestamp > 0 && block.timestamp > prevTimestamp) { + uint32 elapsed = uint32(block.timestamp - prevTimestamp); + uint32[] memory secondsAgos = new uint32[](2); + secondsAgos[0] = elapsed; + secondsAgos[1] = 0; + try pool.observe(secondsAgos) returns (int56[] memory tickCumulatives, uint160[] memory) { + int24 twapTick = int24((tickCumulatives[1] - tickCumulatives[0]) / int56(int32(elapsed))); + return _priceAtTick(token0isWeth ? -1 * twapTick : twapTick); + } catch { + // Observation history not deep enough — fall through to anchor midpoint + } + } + // Fallback: anchor midpoint (original single-snapshot behaviour) + int24 tick = tickLower + ((tickUpper - tickLower) / 2); + priceX96 = _priceAtTick(token0isWeth ? -1 * tick : tick); + } + /// @notice Allow contract to receive ETH receive() external payable { } diff --git a/onchain/src/VWAPTracker.sol b/onchain/src/VWAPTracker.sol index f2cc930..9a93a92 100644 --- a/onchain/src/VWAPTracker.sol +++ b/onchain/src/VWAPTracker.sol @@ -6,13 +6,17 @@ import "@openzeppelin/utils/math/Math.sol"; /** * @title VWAPTracker * @notice Abstract contract for tracking Volume Weighted Average Price (VWAP) data - * @dev Provides VWAP calculation and storage functionality that can be inherited by other contracts + * @dev Provides VWAP calculation and storage functionality that can be inherited by other contracts. + * Price inputs are sourced from the Uniswap V3 pool TWAP oracle (pool.observe()) rather than + * the anchor position midpoint, giving per-second granularity and manipulation resistance. + * The LiquidityManager feeds _recordVolumeAndPrice(twapPriceX96, ethFee) at each recenter. * * Key features: * - Volume-weighted average with data compression (max 1000x compression) * - Prevents dormant whale manipulation through historical price memory * - Stores price² (squared price) in X96 format for VWAP calculation * - Automatic overflow protection by compressing historic data when needed + * - Price source: pool TWAP oracle (time-weighted, per-second) not anchor midpoint snapshot */ abstract contract VWAPTracker { using Math for uint256; diff --git a/onchain/test/VWAPFloorProtection.t.sol b/onchain/test/VWAPFloorProtection.t.sol index 20d099f..df1bbaa 100644 --- a/onchain/test/VWAPFloorProtection.t.sol +++ b/onchain/test/VWAPFloorProtection.t.sol @@ -19,6 +19,7 @@ import { LiquidityManager } from "../src/LiquidityManager.sol"; import { ThreePositionStrategy } from "../src/abstracts/ThreePositionStrategy.sol"; import { TestEnvironment } from "./helpers/TestBase.sol"; import { UniSwapHelper } from "./helpers/UniswapTestBase.sol"; +import "@aperture/uni-v3-lib/TickMath.sol"; contract VWAPFloorProtectionTest is UniSwapHelper { address constant RECENTER_CALLER = address(0x7777); @@ -212,6 +213,105 @@ contract VWAPFloorProtectionTest is UniSwapHelper { } } + // ========================================================================= + // TWAP: price reflects average across interval, not just last swap + // ========================================================================= + + /** + * @notice TWAP oracle gives an average price over the recenter interval, + * not merely the last-swap anchor midpoint. + * + * Sequence: + * 1. First recenter → positions set, no fees (lastRecenterTimestamp = t0). + * 2. Warp 100 s → buy KRK: price moves UP, observation written at t0+100. + * 3. Warp 100 s → buy KRK again: price moves further UP, observation at t0+200. + * 4. Warp 100 s → bootstrap recenter (cumulativeVolume==0 → always records). + * elapsed = 300 s; pool.observe([300,0]) gives TWAP over the full interval. + * + * The TWAP covers 100 s at initial price + 100 s at P_mid + 100 s at P_high. + * The old anchor-midpoint approach would record only the initial anchor tick + * (placed during step 1 before any buys occurred). + * Therefore TWAP-based VWAP > initial-anchor-midpoint VWAP because it accounts + * for the price appreciation during the interval. + */ + function test_twapReflectsAveragePriceNotJustLastSwap() public { + // Note: in Foundry, `block.timestamp` within the test function always returns the + // value at test-function entry (1). vm.warp() takes effect for external calls, so + // we track elapsed time with a local variable. + + // Step 1: initial recenter — places positions at the pool's current price. + // No fees yet; lastRecenterTimestamp is set to block.timestamp. + (, int24 initialTick,,,,,) = pool.slot0(); + vm.prank(RECENTER_CALLER); + lm.recenter(); + + assertEq(lm.cumulativeVolume(), 0, "no fees before first buy"); + + uint256 t = 1; // track warped time independently + + // Step 2: advance 100 s, then buy (price rises; observation written for prior period). + t += 100; + vm.warp(t); // t = 101 + buyRaw(25 ether); + + // Step 3: advance another 100 s, buy again (price rises further). + t += 100; + vm.warp(t); // t = 201 + buyRaw(25 ether); + + // Capture the current (elevated) tick after two rounds of buying. + (, int24 elevatedTick,,,,,) = pool.slot0(); + + // The price must have risen — sanity check for !token0isWeth ordering. + // For !token0isWeth: buying KRK increases the tick (KRK price in WETH rises). + assertFalse(token0isWeth, "test assumes token0isWeth=false"); + assertGt(elevatedTick, initialTick, "price must have risen after buys"); + + // Step 4: advance 100 s then do the bootstrap recenter. + // cumulativeVolume == 0, so shouldRecordVWAP = true regardless of direction. + // elapsed = 300 s → pool.observe([300, 0]) → TWAP tick ≈ avg of three 100-s periods. + t += 100; + vm.warp(t); // t = 301 + uint256 vwapBefore = lm.getVWAP(); + vm.prank(RECENTER_CALLER); + try lm.recenter() { + uint256 vwapAfter = lm.getVWAP(); + + // If fees were collected, VWAP was updated. + if (vwapAfter > 0 && vwapAfter != vwapBefore) { + // TWAP over the 300-s window reflects higher prices than the initial anchor tick. + // The initial anchor was placed at `initialTick` (before any buys). + // TWAP tick ≈ (initialTick·100 + midTick·100 + elevatedTick·100) / 300 > initialTick. + // Correspondingly, priceX96(TWAP) > priceX96(initialTick). + // + // Compute a reference: the price at the initial anchor tick. + // For !token0isWeth, _priceAtTick uses the tick directly (no negation). + // We approximate it via TickMath: sqrtRatio² >> 96. + uint160 sqrtAtInitial = uint160(uint256(TickMath.getSqrtRatioAtTick(initialTick))); + uint256 initialPriceX96 = uint256(sqrtAtInitial) * uint256(sqrtAtInitial) >> 96; + + assertGt( + vwapAfter, + initialPriceX96, + "TWAP VWAP must exceed initial-anchor-midpoint price" + ); + } else if (lm.cumulativeVolume() == 0) { + // No ETH fees collected: ethFee == 0 so _recordVolumeAndPrice was skipped. + // This can happen when feeDestination receives all fees before recording. + // Accept the result as long as VWAP is still 0 (nothing recorded yet). + assertEq(vwapAfter, 0, "VWAP still zero when no ETH fees collected"); + } + } catch (bytes memory reason) { + // Only "amplitude not reached" is an acceptable failure — it means the second + // recenter couldn't detect sufficient price movement relative to the first one. + assertEq( + keccak256(reason), + keccak256(abi.encodeWithSignature("Error(string)", "amplitude not reached.")), + "unexpected revert in bootstrap recenter" + ); + } + } + // ========================================================================= // getLiquidityManager override for UniSwapHelper boundary helpers // ========================================================================= From 83e43386bcfa1cc0e7616d85d0e916c6568abdfd Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 11 Mar 2026 08:30:27 +0000 Subject: [PATCH 2/3] ci: retrigger after infra failure From f72f99aefa0028bcf5a9dda2d4ca6fe5e1568cde Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 11 Mar 2026 10:02:47 +0000 Subject: [PATCH 3/3] fix: Address review findings from PR #575 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix unsafe int32 intermediate cast: int56(int32(elapsed)) → int56(uint56(elapsed)) to prevent TWAP tick sign inversion for intervals above int32 max (~68 years) - Remove redundant lastRecenterTimestamp state variable; capture prevTimestamp from existing lastRecenterTime instead (saves ~20k gas per recenter) - Use pool.increaseObservationCardinalityNext(ORACLE_CARDINALITY) in constructor instead of recomputing the pool address; extract magic 100 to named constant - Add TWAPFallback(uint32 elapsed) event emitted when pool.observe() reverts so monitoring can distinguish degraded operation from normal bootstrap - Remove conditional bypass paths in test_twapReflectsAveragePriceNotJustLastSwap; assert vwapAfter > 0 and vwapAfter > initialPriceX96 unconditionally Co-Authored-By: Claude Sonnet 4.6 --- onchain/src/LiquidityManager.sol | 24 ++++++----- onchain/test/VWAPFloorProtection.t.sol | 56 ++++++++------------------ 2 files changed, 30 insertions(+), 50 deletions(-) diff --git a/onchain/src/LiquidityManager.sol b/onchain/src/LiquidityManager.sol index 617ed78..f16d8aa 100644 --- a/onchain/src/LiquidityManager.sol +++ b/onchain/src/LiquidityManager.sol @@ -51,16 +51,19 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { /// @notice Last recenter tick — used to determine net trade direction between recenters int24 public lastRecenterTick; - /// @notice Last recenter timestamp — rate limits open (permissionless) recenters + /// @notice Last recenter timestamp — rate limits open recenters and provides the previous + /// recenter time for TWAP interval calculations. uint256 public lastRecenterTime; /// @notice Minimum seconds between open recenters (when recenterAccess is unset) uint256 internal constant MIN_RECENTER_INTERVAL = 60; - - /// @notice Last recenter block timestamp — used to compute elapsed interval for pool TWAP oracle - uint256 public lastRecenterTimestamp; + /// @notice Target observation cardinality requested from the pool during construction + uint16 internal constant ORACLE_CARDINALITY = 100; /// @notice Emitted on each successful recenter for monitoring and indexing event Recentered(int24 indexed currentTick, bool indexed isUp); + /// @notice Emitted when pool.observe() falls back to anchor midpoint; non-zero elapsed + /// indicates degraded oracle operation rather than normal bootstrap. + event TWAPFallback(uint32 elapsed); /// @notice Custom errors error ZeroAddressInSetter(); @@ -88,7 +91,7 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { optimizer = Optimizer(_optimizer); // Increase observation cardinality so pool.observe() has sufficient history // for TWAP calculations between recenters. - IUniswapV3Pool(PoolAddress.computeAddress(factory, poolKey)).increaseObservationCardinalityNext(100); + pool.increaseObservationCardinalityNext(ORACLE_CARDINALITY); } /// @notice Callback function for Uniswap V3 mint operations @@ -147,9 +150,8 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { require(block.timestamp >= lastRecenterTime + MIN_RECENTER_INTERVAL, "recenter cooldown"); require(_isPriceStable(currentTick), "price deviated from oracle"); } - uint256 prevTimestamp = lastRecenterTimestamp; + uint256 prevTimestamp = lastRecenterTime; lastRecenterTime = block.timestamp; - lastRecenterTimestamp = block.timestamp; // Check if price movement is sufficient for recentering isUp = false; @@ -283,7 +285,6 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { /// @return priceX96 Price in Q96 format (price * 2^96) function _getTWAPOrFallback(uint256 prevTimestamp, int24 tickLower, int24 tickUpper) internal - view returns (uint256 priceX96) { // Only attempt TWAP when there is a measurable elapsed interval @@ -293,10 +294,13 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { secondsAgos[0] = elapsed; secondsAgos[1] = 0; try pool.observe(secondsAgos) returns (int56[] memory tickCumulatives, uint160[] memory) { - int24 twapTick = int24((tickCumulatives[1] - tickCumulatives[0]) / int56(int32(elapsed))); + int24 twapTick = int24((tickCumulatives[1] - tickCumulatives[0]) / int56(uint56(elapsed))); return _priceAtTick(token0isWeth ? -1 * twapTick : twapTick); } catch { - // Observation history not deep enough — fall through to anchor midpoint + // pool.observe() failed — emit event so monitoring can distinguish + // degraded oracle operation from normal bootstrap (elapsed == 0). + emit TWAPFallback(elapsed); + // Fall through to anchor midpoint } } // Fallback: anchor midpoint (original single-snapshot behaviour) diff --git a/onchain/test/VWAPFloorProtection.t.sol b/onchain/test/VWAPFloorProtection.t.sol index df1bbaa..eb0d9bb 100644 --- a/onchain/test/VWAPFloorProtection.t.sol +++ b/onchain/test/VWAPFloorProtection.t.sol @@ -222,7 +222,7 @@ contract VWAPFloorProtectionTest is UniSwapHelper { * not merely the last-swap anchor midpoint. * * Sequence: - * 1. First recenter → positions set, no fees (lastRecenterTimestamp = t0). + * 1. First recenter → positions set, no fees (lastRecenterTime = t0). * 2. Warp 100 s → buy KRK: price moves UP, observation written at t0+100. * 3. Warp 100 s → buy KRK again: price moves further UP, observation at t0+200. * 4. Warp 100 s → bootstrap recenter (cumulativeVolume==0 → always records). @@ -240,8 +240,9 @@ contract VWAPFloorProtectionTest is UniSwapHelper { // we track elapsed time with a local variable. // Step 1: initial recenter — places positions at the pool's current price. - // No fees yet; lastRecenterTimestamp is set to block.timestamp. + // No fees yet; lastRecenterTime is set to block.timestamp. (, int24 initialTick,,,,,) = pool.slot0(); + assertFalse(token0isWeth, "test assumes token0isWeth=false"); vm.prank(RECENTER_CALLER); lm.recenter(); @@ -261,10 +262,7 @@ contract VWAPFloorProtectionTest is UniSwapHelper { // Capture the current (elevated) tick after two rounds of buying. (, int24 elevatedTick,,,,,) = pool.slot0(); - - // The price must have risen — sanity check for !token0isWeth ordering. // For !token0isWeth: buying KRK increases the tick (KRK price in WETH rises). - assertFalse(token0isWeth, "test assumes token0isWeth=false"); assertGt(elevatedTick, initialTick, "price must have risen after buys"); // Step 4: advance 100 s then do the bootstrap recenter. @@ -272,44 +270,22 @@ contract VWAPFloorProtectionTest is UniSwapHelper { // elapsed = 300 s → pool.observe([300, 0]) → TWAP tick ≈ avg of three 100-s periods. t += 100; vm.warp(t); // t = 301 - uint256 vwapBefore = lm.getVWAP(); vm.prank(RECENTER_CALLER); - try lm.recenter() { - uint256 vwapAfter = lm.getVWAP(); + lm.recenter(); - // If fees were collected, VWAP was updated. - if (vwapAfter > 0 && vwapAfter != vwapBefore) { - // TWAP over the 300-s window reflects higher prices than the initial anchor tick. - // The initial anchor was placed at `initialTick` (before any buys). - // TWAP tick ≈ (initialTick·100 + midTick·100 + elevatedTick·100) / 300 > initialTick. - // Correspondingly, priceX96(TWAP) > priceX96(initialTick). - // - // Compute a reference: the price at the initial anchor tick. - // For !token0isWeth, _priceAtTick uses the tick directly (no negation). - // We approximate it via TickMath: sqrtRatio² >> 96. - uint160 sqrtAtInitial = uint160(uint256(TickMath.getSqrtRatioAtTick(initialTick))); - uint256 initialPriceX96 = uint256(sqrtAtInitial) * uint256(sqrtAtInitial) >> 96; + // TWAP over the 300-s window reflects higher prices than the initial anchor tick. + // TWAP tick ≈ (initialTick·100 + midTick·100 + elevatedTick·100) / 300 > initialTick. + // Correspondingly, priceX96(TWAP) > priceX96(initialTick). + // + // Compute a reference: the price at the initial anchor tick. + // For !token0isWeth, _priceAtTick uses the tick directly (no negation). + // We approximate it via TickMath: sqrtRatio² >> 96. + uint256 vwapAfter = lm.getVWAP(); + assertGt(vwapAfter, 0, "VWAP must be bootstrapped after fees from two large buys"); - assertGt( - vwapAfter, - initialPriceX96, - "TWAP VWAP must exceed initial-anchor-midpoint price" - ); - } else if (lm.cumulativeVolume() == 0) { - // No ETH fees collected: ethFee == 0 so _recordVolumeAndPrice was skipped. - // This can happen when feeDestination receives all fees before recording. - // Accept the result as long as VWAP is still 0 (nothing recorded yet). - assertEq(vwapAfter, 0, "VWAP still zero when no ETH fees collected"); - } - } catch (bytes memory reason) { - // Only "amplitude not reached" is an acceptable failure — it means the second - // recenter couldn't detect sufficient price movement relative to the first one. - assertEq( - keccak256(reason), - keccak256(abi.encodeWithSignature("Error(string)", "amplitude not reached.")), - "unexpected revert in bootstrap recenter" - ); - } + uint160 sqrtAtInitial = uint160(uint256(TickMath.getSqrtRatioAtTick(initialTick))); + uint256 initialPriceX96 = uint256(sqrtAtInitial) * uint256(sqrtAtInitial) >> 96; + assertGt(vwapAfter, initialPriceX96, "TWAP VWAP must exceed initial-anchor-midpoint price"); } // =========================================================================