harb/onchain/test/VWAPFloorProtection.t.sol
openhands f72f99aefa fix: Address review findings from PR #575
- 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 <noreply@anthropic.com>
2026-03-11 10:02:47 +00:00

298 lines
13 KiB
Solidity

// 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";
import "@aperture/uni-v3-lib/TickMath.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"
);
}
}
// =========================================================================
// 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 (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).
* 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; lastRecenterTime is set to block.timestamp.
(, int24 initialTick,,,,,) = pool.slot0();
assertFalse(token0isWeth, "test assumes token0isWeth=false");
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();
// For !token0isWeth: buying KRK increases the tick (KRK price in WETH rises).
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
vm.prank(RECENTER_CALLER);
lm.recenter();
// 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");
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");
}
// =========================================================================
// getLiquidityManager override for UniSwapHelper boundary helpers
// =========================================================================
function getLiquidityManager() external view override returns (ThreePositionStrategy) {
return ThreePositionStrategy(address(lm));
}
}