johba/harb
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request 'fix: shouldRecordVWAP else-branch fires incorrectly when lastRecenterTick==0 after bootstrap (#609)' (#1102) from fix/issue-609 into master

Browse source
This commit is contained in:
johba 2026-03-22 09:06:03 +01:00
commit 5c9cf81589
2 changed files with 144 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

23
onchain/src/LiquidityManager.sol
View file

@ -55,6 +55,14 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle {
/// @notice Last recenter tick — used to determine net trade direction between recenters
int24 public lastRecenterTick;
/// @notice True once lastRecenterTick has been explicitly set by a recenter.
/// @dev Prevents the else-branch direction check from comparing against the
/// Solidity-default 0 when no recenter has established a reference tick.
/// Without this guard, lastRecenterTick==0 is ambiguous: it could mean
/// "never recentered" or "previous recenter landed at tick 0 (price=1.0)".
/// See issue #609.
bool private _hasRecenterTick;
/// @notice Last recenter timestamp — rate limits recenters.
uint256 public lastRecenterTime;
/// @notice Minimum seconds between recenters
@ -202,16 +210,25 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle {
// 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
if (!_hasRecenterTick || cumulativeVolume == 0) {
// No reference tick yet, or no VWAP data — always bootstrap to prevent
// vwapX96=0 fallback. The _hasRecenterTick guard (issue #609) ensures we
// never run the direction comparison against the Solidity-default 0 before
// a recenter has established a real reference point.
shouldRecordVWAP = true;
} else {
// 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.
// Only record when price falls — VWAP stays anchored to historical levels
// during buy attacks.
//
// Edge case (issue #609): when lastRecenterTick==0 because a previous recenter
// landed at tick 0 (price = 1.0 token ratio), the comparison below correctly
// identifies direction — tick 0 is a valid reference like any other tick.
shouldRecordVWAP = token0isWeth ? (currentTick > lastRecenterTick) : (currentTick < lastRecenterTick);
}
lastRecenterTick = currentTick;
_hasRecenterTick = true;
_scrapePositions(shouldRecordVWAP, currentTick);

124
onchain/test/VWAPFloorProtection.t.sol
View file

@ -263,6 +263,130 @@ contract VWAPFloorProtectionTest is UniSwapHelper {
assertGt(lm.getVWAP(), 0, "seed trade must anchor VWAP to the real launch price");
}
// =========================================================================
// Issue #609: tick-0 edge case — _hasRecenterTick guard
// =========================================================================
/**
* @notice Verifies that the _hasRecenterTick guard prevents the direction
* comparison from running against the Solidity-default lastRecenterTick==0
* before any recenter has established a valid reference tick.
*
* Before fix #609, the bootstrap condition was `cumulativeVolume == 0`.
* Both conditions produce identical behaviour in practice (lastRecenterTick is
* always set before cumulativeVolume can become positive), but the explicit
* _hasRecenterTick flag makes the intent clear and prevents regressions if the
* volume-recording path is ever refactored.
*
* This test mirrors the deployment bootstrap sequence and then runs buy-only
* cycles, asserting that VWAP remains frozen throughout — confirming that the
* direction check (which now only fires after _hasRecenterTick is true) still
* correctly blocks VWAP inflation during buy-only attacks.
*/
function test_hasRecenterTickGuardPreventsTick0Ambiguity() public {
// Before any recenter: lastRecenterTick == 0 (Solidity default).
// The _hasRecenterTick flag must be false, so the bootstrap path fires
// on the first recenter regardless of the default-0 comparison.
assertEq(lm.lastRecenterTick(), 0, "lastRecenterTick defaults to 0 before first recenter");
// ---- step 1: first recenter — positions deployed, no fees ----
vm.prank(RECENTER_CALLER);
lm.recenter();
// lastRecenterTick is now set to the actual pool tick (far from 0).
// _hasRecenterTick is true after this call.
int24 tickAfterFirstRecenter = lm.lastRecenterTick();
assertTrue(tickAfterFirstRecenter != 0, "pool tick should not be exactly 0 after init");
// ---- step 2: buy + recenter → bootstrap VWAP ----
buyRaw(25 ether);
vm.warp(block.timestamp + 301);
vm.prank(RECENTER_CALLER);
lm.recenter();
uint256 bootstrapVWAP = lm.getVWAP();
assertGt(bootstrapVWAP, 0, "VWAP must be bootstrapped");
assertGt(lm.cumulativeVolume(), 0, "cumulativeVolume must be positive after bootstrap");
// ---- step 3: buy-only attack cycles ----
uint256 successfulBuyCycles;
uint256 ts = block.timestamp;
for (uint256 i = 0; i < 8; i++) {
buyRaw(25 ether);
ts += 301;
vm.warp(ts);
vm.prank(RECENTER_CALLER);
try lm.recenter() {
successfulBuyCycles++;
} catch { }
}
assertGt(successfulBuyCycles, 0, "at least one buy-recenter cycle must succeed");
// ---- step 4: VWAP must remain frozen ----
assertEq(
lm.getVWAP(),
bootstrapVWAP,
"issue #609: VWAP must stay frozen during buy-only cycles (direction check + _hasRecenterTick guard)"
);
}
/**
* @notice After a sell-then-buy sequence where lastRecenterTick could land
* near zero, buy-only cycles must still not inflate VWAP.
*
* This test exercises the direction comparison in the else-branch after
* multiple recenters have occurred, verifying correctness regardless of
* the specific lastRecenterTick value.
*/
function test_vwapFrozenDuringBuyOnlyAfterSellRecenter() public {
// Bootstrap
vm.prank(RECENTER_CALLER);
lm.recenter();
buyRaw(25 ether);
vm.warp(block.timestamp + 301);
vm.prank(RECENTER_CALLER);
lm.recenter();
uint256 vwapAfterBootstrap = lm.getVWAP();
assertGt(vwapAfterBootstrap, 0, "VWAP bootstrapped");
// Sell back to move price down — triggers VWAP recording (sell direction)
uint256 harbBal = harberg.balanceOf(account);
if (harbBal > 0) {
sellRaw(harbBal);
}
vm.warp(block.timestamp + 301);
vm.prank(RECENTER_CALLER);
try lm.recenter() { } catch { }
// Snapshot VWAP after sell-direction update
uint256 vwapAfterSell = lm.getVWAP();
// Now run buy-only cycles — VWAP must not increase
uint256 ts = block.timestamp;
uint256 successfulBuyCycles;
for (uint256 i = 0; i < 6; i++) {
buyRaw(25 ether);
ts += 301;
vm.warp(ts);
vm.prank(RECENTER_CALLER);
try lm.recenter() {
successfulBuyCycles++;
} catch { }
}
if (successfulBuyCycles > 0) {
assertEq(
lm.getVWAP(),
vwapAfterSell,
"VWAP must stay frozen during buy-only cycles after sell-direction recenter"
);
}
}
// =========================================================================
// getLiquidityManager override for UniSwapHelper boundary helpers
// =========================================================================