From b8f5ed9411970bce5df5981c4bde948b91543d93 Mon Sep 17 00:00:00 2001 From: openhands Date: Thu, 12 Mar 2026 19:21:11 +0000 Subject: [PATCH] fix: Dead code branch: lastRecenterTick==0 with cumulativeVolume>0 (#568) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove unreachable else branch in VWAP recording logic. The branch was only reachable if lastRecenterTick==0 and cumulativeVolume>0, which requires tick==0 on the very first recenter — virtually impossible on a live pool. Collapse else-if into else and delete the corresponding testVWAPElseBranch test that exercised the path via vm.store. Co-Authored-By: Claude Sonnet 4.6 --- onchain/src/LiquidityManager.sol | 5 +---- onchain/test/LiquidityManager.t.sol | 27 --------------------------- 2 files changed, 1 insertion(+), 31 deletions(-) diff --git a/onchain/src/LiquidityManager.sol b/onchain/src/LiquidityManager.sol index 91e2787..70eadac 100644 --- a/onchain/src/LiquidityManager.sol +++ b/onchain/src/LiquidityManager.sol @@ -186,14 +186,11 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { if (cumulativeVolume == 0) { // No VWAP data yet — always bootstrap to prevent vwapX96=0 fallback shouldRecordVWAP = true; - } else if (lastRecenterTick != 0) { + } 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. shouldRecordVWAP = token0isWeth ? (currentTick > lastRecenterTick) : (currentTick < lastRecenterTick); - } else { - // First recenter — no reference point, record conservatively - shouldRecordVWAP = true; } lastRecenterTick = currentTick; diff --git a/onchain/test/LiquidityManager.t.sol b/onchain/test/LiquidityManager.t.sol index 5ae78bf..3fe09cf 100644 --- a/onchain/test/LiquidityManager.t.sol +++ b/onchain/test/LiquidityManager.t.sol @@ -1084,33 +1084,6 @@ contract LiquidityManagerTest is UniSwapHelper { assertGt(anchorLiq, 0, "ANCHOR position should have been created with fallback params"); } - /** - * @notice VWAP else branch: cumulativeVolume > 0 AND lastRecenterTick == 0 - * Reached by re-deploying a fresh LM (lastRecenterTick = 0) and setting - * cumulativeVolume via vm.store before the first recenter. - */ - function testVWAPElseBranch() public { - // Re-deploy fresh protocol so lastRecenterTick starts at 0 - deployProtocolWithTokenOrder(DEFAULT_TOKEN0_IS_WETH); - - // Preconditions: fresh LM has lastRecenterTick == 0 and cumulativeVolume == 0 - assertEq(lm.lastRecenterTick(), 0, "precondition: lastRecenterTick must be 0"); - assertEq(lm.cumulativeVolume(), 0, "precondition: cumulativeVolume must be 0"); - - // Set cumulativeVolume to non-zero (storage slot 1 in VWAPTracker). - // This creates the state: cumulativeVolume > 0 AND lastRecenterTick == 0, - // which triggers the else branch (line 170) on the first recenter. - vm.store(address(lm), bytes32(uint256(1)), bytes32(uint256(1e18))); - assertEq(lm.cumulativeVolume(), 1e18, "cumulativeVolume should be 1e18 after vm.store"); - - // Call recenter — hits the else branch, sets lastRecenterTick, then sets positions - vm.prank(RECENTER_CALLER); - lm.recenter(); - - // Verify lastRecenterTick was updated from the recenter - assertTrue(lm.lastRecenterTick() != 0, "lastRecenterTick should have been updated after recenter"); - } - /** * @notice When feeDestination == address(lm), recenter must not transfer fees externally * and _getOutstandingSupply must not double-subtract LM-held KRK.