diff --git a/onchain/AGENTS.md b/onchain/AGENTS.md index b4315e3..ce6ad0c 100644 --- a/onchain/AGENTS.md +++ b/onchain/AGENTS.md @@ -42,7 +42,7 @@ The staking system traces a triangle in (staking%, avgTax) space: ## System Snapshot - Kraiken ERC20 with mint/burn via LiquidityManager. VERSION=2. - LiquidityManager.sol: ANCHOR + DISCOVERY + FLOOR positions with asymmetric slippage. -- VWAPTracker.sol: squared price in X96, compression, directional recording (ETH inflow only). +- VWAPTracker.sol: squared price in X96, compression, directional recording (price-fall / ETH-outflow events only — buy-only cycles must NOT update VWAP or the floor tracks the inflated price, crystallising IL; see issue #543). - OptimizerV3.sol: UUPS upgradeable, direct 2D binary mapping. - Stake.sol: self-assessed tax, snatching auctions, discrete brackets, UBI redistribution. diff --git a/onchain/src/LiquidityManager.sol b/onchain/src/LiquidityManager.sol index 3786e35..6efda70 100644 --- a/onchain/src/LiquidityManager.sol +++ b/onchain/src/LiquidityManager.sol @@ -156,15 +156,21 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { } // Remove all existing positions and collect fees - // Pass tick direction to determine if VWAP should record (ETH inflow only) + // Pass tick direction to determine if VWAP should record. + // CRITICAL: record VWAP only when price FALLS (sells / ETH outflow), never when it rises. + // If we recorded during buy events, an adversary could run N buy-recenter cycles to push + // VWAP upward toward the inflated price. When VWAP ≈ current tick, mirrorTick ≈ currentTick, + // so the floor is placed near the inflated price — crystallising IL when the adversary sells. + // Freezing VWAP during buy-only cycles keeps the floor anchored to the historical baseline. bool shouldRecordVWAP; if (cumulativeVolume == 0) { // No VWAP data yet — always bootstrap to prevent vwapX96=0 fallback shouldRecordVWAP = true; } else if (lastRecenterTick != 0) { - // token0isWeth: tick DOWN = price up in KRK terms = buys = ETH inflow - // !token0isWeth: tick UP = price up in KRK terms = buys = ETH inflow - shouldRecordVWAP = token0isWeth ? (currentTick < lastRecenterTick) : (currentTick > lastRecenterTick); + // token0isWeth: tick UP = price down in KRK terms = sells = ETH outflow + // !token0isWeth: tick DOWN = price down in KRK terms = sells = ETH outflow + // Only record when price falls — VWAP stays anchored to historical levels during buy attacks. + shouldRecordVWAP = token0isWeth ? (currentTick > lastRecenterTick) : (currentTick < lastRecenterTick); } else { // First recenter — no reference point, record conservatively shouldRecordVWAP = true; diff --git a/onchain/test/VWAPFloorProtection.t.sol b/onchain/test/VWAPFloorProtection.t.sol new file mode 100644 index 0000000..b1f9331 --- /dev/null +++ b/onchain/test/VWAPFloorProtection.t.sol @@ -0,0 +1,222 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.19; + +/** + * @title VWAPFloorProtectionTest + * @notice Regression tests for issue #543: VWAP must not inflate during buy-only recenter cycles. + * + * Root cause (pre-fix): shouldRecordVWAP was true on BUY events (ETH inflow). An adversary + * running N buy-recenter cycles continuously updated VWAP upward toward the current (inflated) + * price. When VWAP ≈ currentTick the mirrorTick formula placed the floor near the current + * price, crystallising IL when the adversary finally sold all KRK. + * + * Fix: shouldRecordVWAP is now true only on SELL events (price falling / ETH outflow). + * Buy-only cycles leave VWAP frozen at the historical bootstrap level, keeping the floor + * conservatively anchored to that baseline. + */ + +import { LiquidityManager } from "../src/LiquidityManager.sol"; +import { ThreePositionStrategy } from "../src/abstracts/ThreePositionStrategy.sol"; +import { Kraiken } from "../src/Kraiken.sol"; +import "../src/interfaces/IWETH9.sol"; +import { TestEnvironment } from "./helpers/TestBase.sol"; +import { UniSwapHelper } from "./helpers/UniswapTestBase.sol"; +import "@uniswap-v3-core/interfaces/IUniswapV3Pool.sol"; +import "forge-std/Test.sol"; + +contract VWAPFloorProtectionTest is UniSwapHelper { + address constant RECENTER_CALLER = address(0x7777); + + LiquidityManager lm; + TestEnvironment testEnv; + address feeDestination = makeAddr("fees"); + + // How much ETH to give the LM and the attacker + uint256 constant LM_ETH = 100 ether; + uint256 constant ATTACKER_ETH = 2000 ether; + + function setUp() public { + testEnv = new TestEnvironment(feeDestination); + (,pool, weth, harberg, , lm, , token0isWeth) = + testEnv.setupEnvironment(false, RECENTER_CALLER); + + vm.deal(address(lm), LM_ETH); + + // Fund the swap account used by UniSwapHelper + vm.deal(account, ATTACKER_ETH); + vm.prank(account); + weth.deposit{ value: ATTACKER_ETH / 2 }(); + } + + // ========================================================================= + // Core regression: VWAP must not inflate during buy-only cycles (#543) + // ========================================================================= + + /** + * @notice VWAP stays at its bootstrap value throughout a buy-only attack sequence. + * + * Sequence: + * 1. First recenter → creates positions (no fees yet, VWAP not recorded). + * 2. Buy KRK (price rises) → second recenter → bootstraps VWAP (cumulativeVolume == 0). + * 3. Repeat buy + recenter several more times. + * 4. Assert getVWAP() is unchanged from the bootstrap recording. + * + * Before the fix this test would fail because every successful buy-direction recenter + * updated VWAP with the new (higher) anchor price, pulling VWAP toward currentTick. + */ + function test_vwapNotInflatedByBuyOnlyAttack() public { + // ---- step 1: initial recenter sets up positions ---- + vm.prank(RECENTER_CALLER); + lm.recenter(); + + assertEq(lm.cumulativeVolume(), 0, "no fees collected yet: cumulativeVolume == 0"); + + // ---- step 2: first buy + recenter → bootstrap ---- + buyRaw(25 ether); // push price up enough to satisfy amplitude check + + vm.prank(RECENTER_CALLER); + lm.recenter(); // cumulativeVolume == 0 → shouldRecordVWAP = true (bootstrap path) + + uint256 bootstrapVWAP = lm.getVWAP(); + assertGt(bootstrapVWAP, 0, "VWAP must be recorded at bootstrap"); + + // ---- step 3: continued buy-only cycles ---- + uint256 successfulBuyCycles; + for (uint256 i = 0; i < 10; i++) { + buyRaw(25 ether); + vm.prank(RECENTER_CALLER); + // Recenter may fail if amplitude isn't reached; that's fine. + try lm.recenter() { + successfulBuyCycles++; + } catch { } + } + + // Ensure at least some cycles succeeded so the test is meaningful. + assertGt(successfulBuyCycles, 0, "at least one buy-recenter cycle must succeed"); + + // ---- step 4: VWAP must be unchanged ---- + uint256 vwapAfterAttack = lm.getVWAP(); + assertEq( + vwapAfterAttack, + bootstrapVWAP, + "VWAP must remain frozen at bootstrap value during buy-only cycles" + ); + } + + /** + * @notice The floor is anchored conservatively (far from the inflated current price) + * after a buy-only attack, making extraction unprofitable. + * + * After N buy cycles the current tick is far above the bootstrap. With VWAP frozen at + * bootstrap, mirrorTick ≈ vwapTick ≈ bootstrapTick — much further from currentTick than + * if VWAP had tracked upward. The floor therefore sits near the original distribution + * price, not the inflated price. + */ + function test_floorConservativeAfterBuyOnlyAttack() public { + vm.prank(RECENTER_CALLER); + lm.recenter(); + + // Bootstrap via first buy-recenter + buyRaw(25 ether); + vm.prank(RECENTER_CALLER); + lm.recenter(); + + // Run several buy cycles + for (uint256 i = 0; i < 6; i++) { + buyRaw(25 ether); + vm.prank(RECENTER_CALLER); + try lm.recenter() { } catch { } + } + + // Read floor and current tick + (, int24 currentTick,,,,,) = pool.slot0(); + (, int24 floorTickLower, int24 floorTickUpper) = + lm.positions(ThreePositionStrategy.Stage.FLOOR); + + int24 floorCenter = floorTickLower + (floorTickUpper - floorTickLower) / 2; + + // For !token0isWeth buying pushes tick UP (KRK more expensive). + // The floor must be BELOW the current inflated tick by a substantial margin — + // at minimum the anchor spacing plus some additional buffer from VWAP anchoring. + // We assert that the gap is at least 400 ticks (two tick spacings). + int24 gap = currentTick - floorCenter; + assertGt(gap, 400, "floor must be substantially below the inflated current tick"); + } + + // ========================================================================= + // Positive: VWAP bootstrap still works + // ========================================================================= + + /** + * @notice The very first fee event always records VWAP regardless of direction. + * + * cumulativeVolume == 0 triggers unconditional recording to avoid the + * vwapX96 == 0 fallback path. This test confirms that path is preserved. + */ + function test_vwapBootstrapsOnFirstFeeEvent() public { + vm.prank(RECENTER_CALLER); + lm.recenter(); + + assertEq(lm.cumulativeVolume(), 0, "no VWAP data before first fees"); + + buyRaw(25 ether); + + vm.prank(RECENTER_CALLER); + lm.recenter(); + + assertGt(lm.cumulativeVolume(), 0, "bootstrap: cumulativeVolume must be positive"); + assertGt(lm.getVWAP(), 0, "bootstrap: VWAP must be positive after first fees"); + } + + // ========================================================================= + // Positive: VWAP updates when price falls (sell-side events) + // ========================================================================= + + /** + * @notice VWAP can still be updated when price falls between recenters. + * + * Sequence: + * 1. Buy large → recenter (bootstrap VWAP at high price). + * 2. Sell all KRK → price falls below bootstrap. + * 3. Recenter with price-fall direction → shouldRecordVWAP = true. + * 4. If ETH fees were collected (buys happened in the prior cycle), VWAP updates. + * We verify at minimum that the recenter succeeds without reverting — i.e. + * the sell-direction path doesn't break the system. + */ + function test_recenterSucceedsOnSellDirectionWithoutReverts() public { + // Bootstrap + vm.prank(RECENTER_CALLER); + lm.recenter(); + + buyRaw(25 ether); + vm.prank(RECENTER_CALLER); + lm.recenter(); + + // Sell back: harberg balance of `account` from the prior buy + uint256 harbBalance = harberg.balanceOf(account); + if (harbBalance > 0) { + sellRaw(harbBalance); + } + + // Recenter with price now lower (sell direction) — must not revert + vm.prank(RECENTER_CALLER); + try lm.recenter() { + // success — sell-direction recenter works + } catch (bytes memory reason) { + // Amplitude not met is the only acceptable failure + assertEq( + keccak256(reason), + keccak256(abi.encodeWithSignature("Error(string)", "amplitude not reached.")), + "unexpected revert in sell-direction recenter" + ); + } + } + + // ========================================================================= + // getLiquidityManager override for UniSwapHelper boundary helpers + // ========================================================================= + + function getLiquidityManager() external view override returns (ThreePositionStrategy) { + return ThreePositionStrategy(address(lm)); + } +}