Merge pull request 'fix: Investigate: VWAP not preventing IL crystallization during buy-only recenter cycles (#543)' (#566) from fix/issue-543 into master
This commit is contained in:
commit
f70a7e71a2
3 changed files with 235 additions and 8 deletions
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
@ -205,7 +211,7 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle {
|
|||
}
|
||||
|
||||
/// @notice Removes all positions and collects fees
|
||||
/// @param recordVWAP Whether to record VWAP (only when net ETH inflow since last recenter)
|
||||
/// @param recordVWAP Whether to record VWAP (only when net ETH outflow / price fell since last recenter, or at bootstrap)
|
||||
function _scrapePositions(bool recordVWAP) internal {
|
||||
uint256 fee0 = 0;
|
||||
uint256 fee1 = 0;
|
||||
|
|
@ -233,8 +239,7 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle {
|
|||
}
|
||||
|
||||
// Transfer fees and record volume for VWAP
|
||||
// Only record VWAP when net ETH inflow (KRK sold out) — prevents sell-back
|
||||
// activity from diluting the price memory of original KRK distribution.
|
||||
// VWAP is recorded only on sell events (price fell) or at bootstrap — see recenter().
|
||||
// Skip transfer when feeDestination is self — fees accrue as deployable liquidity.
|
||||
if (feeDestination != address(this)) {
|
||||
if (fee0 > 0) {
|
||||
|
|
|
|||
222
onchain/test/VWAPFloorProtection.t.sol
Normal file
222
onchain/test/VWAPFloorProtection.t.sol
Normal file
|
|
@ -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 { TestEnvironment } from "./helpers/TestBase.sol";
|
||||
import { UniSwapHelper } from "./helpers/UniswapTestBase.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 { }
|
||||
}
|
||||
|
||||
// This test is written for token0isWeth=false (set in setUp via setupEnvironment(false,...)).
|
||||
// For !token0isWeth: buying KRK pushes tick UP (WETH/KRK price rises), so floor must be
|
||||
// BELOW (lower tick than) the inflated current tick.
|
||||
assertFalse(token0isWeth, "test assumes token0isWeth=false; update gap logic if changed");
|
||||
|
||||
// Read floor and current tick
|
||||
(, int24 currentTick,,,,,) = pool.slot0();
|
||||
(, int24 floorTickLower, int24 floorTickUpper) =
|
||||
lm.positions(ThreePositionStrategy.Stage.FLOOR);
|
||||
|
||||
int24 floorCenter = floorTickLower + (floorTickUpper - floorTickLower) / 2;
|
||||
|
||||
// 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));
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue