From 7007e593da72fef44ec4e859c5d333008fad17b6 Mon Sep 17 00:00:00 2001 From: openhands Date: Fri, 20 Mar 2026 02:05:25 +0000 Subject: [PATCH 1/2] fix: getLiquidityParams: int256 overflow on large VWAP values (#622) Co-Authored-By: Claude Opus 4.6 (1M context) --- onchain/src/Optimizer.sol | 1 + onchain/test/Optimizer.t.sol | 75 ++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/onchain/src/Optimizer.sol b/onchain/src/Optimizer.sol index 7c596e7..b8ae8a7 100644 --- a/onchain/src/Optimizer.sol +++ b/onchain/src/Optimizer.sol @@ -414,6 +414,7 @@ contract Optimizer is Initializable, UUPSUpgradeable, IOptimizer { // ---- Slot 2: pricePosition (also needs VWAP) ---- if (vwapTracker != address(0)) { uint256 vwapX96 = IVWAPTracker(vwapTracker).getVWAP(); + require(vwapX96 <= uint256(type(int256).max), "VWAP exceeds int256 range"); if (vwapX96 > 0) { // Convert pool tick to KRK-price space: higher tick = more expensive KRK. // Uniswap convention: tick ↑ → token1 more expensive relative to token0. diff --git a/onchain/test/Optimizer.t.sol b/onchain/test/Optimizer.t.sol index c88bf2b..ea09bdd 100644 --- a/onchain/test/Optimizer.t.sol +++ b/onchain/test/Optimizer.t.sol @@ -704,3 +704,78 @@ contract OptimizerNormalizedInputsTest is Test { import { TickMath } from "@aperture/uni-v3-lib/TickMath.sol"; import { Math } from "@openzeppelin/utils/math/Math.sol"; + +// ============================================================================= +// VWAP int256 overflow guard test (issue #622) +// ============================================================================= + +/// @dev Minimal mock that returns a configurable VWAP value for overflow testing. +contract ConfigurableVWAPMock { + uint256 private _vwap; + + function setVWAP(uint256 v) external { + _vwap = v; + } + + function getVWAP() external view returns (uint256) { + return _vwap; + } +} + +contract OptimizerVWAPOverflowTest is Test { + OptimizerInputCapture capture; + Optimizer optimizer; + MockStake mockStake; + MockKraiken mockKraiken; + ConfigurableVWAPMock configurableVwap; + MockPool mockPool; + + function setUp() public { + mockKraiken = new MockKraiken(); + mockStake = new MockStake(); + configurableVwap = new ConfigurableVWAPMock(); + mockPool = new MockPool(); + + OptimizerInputCapture impl = new OptimizerInputCapture(); + bytes memory initData = abi.encodeWithSelector(Optimizer.initialize.selector, address(mockKraiken), address(mockStake)); + ERC1967Proxy proxy = new ERC1967Proxy(address(impl), initData); + capture = OptimizerInputCapture(address(proxy)); + optimizer = Optimizer(address(proxy)); + + optimizer.setDataSources(address(configurableVwap), address(mockPool), address(0), false); + mockPool.setCurrentTick(0); + mockPool.setRevertOnObserve(true); + } + + /// @notice VWAP value exceeding int256.max must revert with descriptive message. + function testRevertsWhenVWAPExceedsInt256Max() public { + configurableVwap.setVWAP(uint256(type(int256).max) + 1); + + vm.expectRevert("VWAP exceeds int256 range"); + optimizer.getLiquidityParams(); + } + + /// @notice VWAP at exactly int256.max should not revert. + function testAcceptsVWAPAtInt256Max() public { + configurableVwap.setVWAP(uint256(type(int256).max)); + + // Should not revert + optimizer.getLiquidityParams(); + } + + /// @notice VWAP at uint256.max must revert. + function testRevertsWhenVWAPIsUint256Max() public { + configurableVwap.setVWAP(type(uint256).max); + + vm.expectRevert("VWAP exceeds int256 range"); + optimizer.getLiquidityParams(); + } + + /// @notice VWAP of zero should not revert (no-op path). + function testAcceptsVWAPZero() public { + configurableVwap.setVWAP(0); + + // Should not revert — zero VWAP skips the pricePosition computation + optimizer.getLiquidityParams(); + } +} From e28e69c98a27eb57ae552133fb67c47fc8575c29 Mon Sep 17 00:00:00 2001 From: openhands Date: Fri, 20 Mar 2026 02:46:30 +0000 Subject: [PATCH 2/2] fix: guard int128 overflow in ThreePositionStrategy mirror tick (#622) Move overflow guard to the actual vulnerable site: ThreePositionStrategy._computeFloorTickWithSignal() line 262 where vwapX96 >> 32 is cast to int128 for _tickAtPriceRatio. Values exceeding int128.max now skip mirror tick (fallback to scarcity/clamp) instead of reverting. Remove incorrect require from Optimizer._buildInputs() which guarded a non-existent int256 cast path. Co-Authored-By: Claude Opus 4.6 (1M context) --- onchain/src/Optimizer.sol | 1 - .../src/abstracts/ThreePositionStrategy.sol | 16 ++-- onchain/test/Optimizer.t.sol | 74 ------------------- .../abstracts/ThreePositionStrategy.t.sol | 68 +++++++++++++++++ 4 files changed, 79 insertions(+), 80 deletions(-) diff --git a/onchain/src/Optimizer.sol b/onchain/src/Optimizer.sol index b8ae8a7..7c596e7 100644 --- a/onchain/src/Optimizer.sol +++ b/onchain/src/Optimizer.sol @@ -414,7 +414,6 @@ contract Optimizer is Initializable, UUPSUpgradeable, IOptimizer { // ---- Slot 2: pricePosition (also needs VWAP) ---- if (vwapTracker != address(0)) { uint256 vwapX96 = IVWAPTracker(vwapTracker).getVWAP(); - require(vwapX96 <= uint256(type(int256).max), "VWAP exceeds int256 range"); if (vwapX96 > 0) { // Convert pool tick to KRK-price space: higher tick = more expensive KRK. // Uniswap convention: tick ↑ → token1 more expensive relative to token0. diff --git a/onchain/src/abstracts/ThreePositionStrategy.sol b/onchain/src/abstracts/ThreePositionStrategy.sol index d3e2c0b..902f285 100644 --- a/onchain/src/abstracts/ThreePositionStrategy.sol +++ b/onchain/src/abstracts/ThreePositionStrategy.sol @@ -259,11 +259,17 @@ abstract contract ThreePositionStrategy is UniswapMath, VWAPTracker { { uint256 vwapX96 = getAdjustedVWAP(params.capitalInefficiency); if (vwapX96 > 0) { - int24 rawVwapTick = _tickAtPriceRatio(int128(int256(vwapX96 >> 32))); - rawVwapTick = token0isWeth ? -rawVwapTick : rawVwapTick; - int24 vwapDistance = currentTick - rawVwapTick; - if (vwapDistance < 0) vwapDistance = -vwapDistance; - mirrorTick = token0isWeth ? currentTick + vwapDistance : currentTick - vwapDistance; + // vwapX96 >> 32 converts Q96 → Q64 (ABDKMath64x64) for _tickAtPriceRatio. + // Skip mirror tick if the shifted value exceeds int128 range to prevent + // overflow in the int128 cast (#622). mirrorTick stays at currentTick. + uint256 priceRatioX64 = vwapX96 >> 32; + if (priceRatioX64 <= uint256(uint128(type(int128).max))) { + int24 rawVwapTick = _tickAtPriceRatio(int128(int256(priceRatioX64))); + rawVwapTick = token0isWeth ? -rawVwapTick : rawVwapTick; + int24 vwapDistance = currentTick - rawVwapTick; + if (vwapDistance < 0) vwapDistance = -vwapDistance; + mirrorTick = token0isWeth ? currentTick + vwapDistance : currentTick - vwapDistance; + } } } diff --git a/onchain/test/Optimizer.t.sol b/onchain/test/Optimizer.t.sol index ea09bdd..18e0728 100644 --- a/onchain/test/Optimizer.t.sol +++ b/onchain/test/Optimizer.t.sol @@ -705,77 +705,3 @@ contract OptimizerNormalizedInputsTest is Test { import { TickMath } from "@aperture/uni-v3-lib/TickMath.sol"; import { Math } from "@openzeppelin/utils/math/Math.sol"; -// ============================================================================= -// VWAP int256 overflow guard test (issue #622) -// ============================================================================= - -/// @dev Minimal mock that returns a configurable VWAP value for overflow testing. -contract ConfigurableVWAPMock { - uint256 private _vwap; - - function setVWAP(uint256 v) external { - _vwap = v; - } - - function getVWAP() external view returns (uint256) { - return _vwap; - } -} - -contract OptimizerVWAPOverflowTest is Test { - OptimizerInputCapture capture; - Optimizer optimizer; - MockStake mockStake; - MockKraiken mockKraiken; - ConfigurableVWAPMock configurableVwap; - MockPool mockPool; - - function setUp() public { - mockKraiken = new MockKraiken(); - mockStake = new MockStake(); - configurableVwap = new ConfigurableVWAPMock(); - mockPool = new MockPool(); - - OptimizerInputCapture impl = new OptimizerInputCapture(); - bytes memory initData = abi.encodeWithSelector(Optimizer.initialize.selector, address(mockKraiken), address(mockStake)); - ERC1967Proxy proxy = new ERC1967Proxy(address(impl), initData); - capture = OptimizerInputCapture(address(proxy)); - optimizer = Optimizer(address(proxy)); - - optimizer.setDataSources(address(configurableVwap), address(mockPool), address(0), false); - mockPool.setCurrentTick(0); - mockPool.setRevertOnObserve(true); - } - - /// @notice VWAP value exceeding int256.max must revert with descriptive message. - function testRevertsWhenVWAPExceedsInt256Max() public { - configurableVwap.setVWAP(uint256(type(int256).max) + 1); - - vm.expectRevert("VWAP exceeds int256 range"); - optimizer.getLiquidityParams(); - } - - /// @notice VWAP at exactly int256.max should not revert. - function testAcceptsVWAPAtInt256Max() public { - configurableVwap.setVWAP(uint256(type(int256).max)); - - // Should not revert - optimizer.getLiquidityParams(); - } - - /// @notice VWAP at uint256.max must revert. - function testRevertsWhenVWAPIsUint256Max() public { - configurableVwap.setVWAP(type(uint256).max); - - vm.expectRevert("VWAP exceeds int256 range"); - optimizer.getLiquidityParams(); - } - - /// @notice VWAP of zero should not revert (no-op path). - function testAcceptsVWAPZero() public { - configurableVwap.setVWAP(0); - - // Should not revert — zero VWAP skips the pricePosition computation - optimizer.getLiquidityParams(); - } -} diff --git a/onchain/test/abstracts/ThreePositionStrategy.t.sol b/onchain/test/abstracts/ThreePositionStrategy.t.sol index 1292d9a..ead0d2f 100644 --- a/onchain/test/abstracts/ThreePositionStrategy.t.sol +++ b/onchain/test/abstracts/ThreePositionStrategy.t.sol @@ -502,4 +502,72 @@ contract ThreePositionStrategyTest is TestConstants { vm.expectRevert(); strategy.setAnchorPosition(CURRENT_TICK, 20 ether, params); } + + // ======================================== + // VWAP INT128 OVERFLOW GUARD (#622) + // ======================================== + + /// @notice VWAP values that would overflow int128 in _tickAtPriceRatio are handled + /// gracefully — floor is placed using scarcity/clamp signals instead of reverting (#622). + function testFloorPositionLargeVWAPNoOverflow() public { + ThreePositionStrategy.PositionParams memory params = getDefaultParams(); + params.capitalInefficiency = 0; // adjustedVWAP = 7*rawVwap/10 + + // Set VWAP so that getAdjustedVWAP(0) >> 32 exceeds int128.max (2^127-1). + // rawVwap = 2^160 → adjustedVwap ≈ 7*2^160/10 ≈ 2^159.1 → shifted ≈ 2^127.1 > int128.max + uint256 hugeVwap = uint256(1) << 160; + strategy.setVWAP(hugeVwap, 1); + + uint256 floorEthBalance = 80 ether; + uint256 pulledHarb = 1000 ether; + uint256 discoveryAmount = 500 ether; + + // Must not revert — mirror tick is skipped, floor uses scarcity/clamp fallback + strategy.setFloorPosition(CURRENT_TICK, floorEthBalance, pulledHarb, discoveryAmount, params); + + MockThreePositionStrategy.MintedPosition memory pos = strategy.getMintedPosition(0); + assertEq(uint256(pos.stage), uint256(ThreePositionStrategy.Stage.FLOOR), "Should be floor position"); + assertTrue(pos.liquidity > 0, "Floor should have liquidity"); + } + + /// @notice VWAP at uint256.max >> 32 boundary — must not revert. + function testFloorPositionMaxVWAPNoOverflow() public { + ThreePositionStrategy.PositionParams memory params = getDefaultParams(); + params.capitalInefficiency = 0; + + // Use a very large VWAP near uint256 limits + // getAdjustedVWAP(0) = 7 * rawVwap / 10 — stays within uint256 + uint256 maxSafeVwap = type(uint256).max / 7; // avoids overflow in 7*rawVwap + strategy.setVWAP(maxSafeVwap, 1); + + uint256 floorEthBalance = 80 ether; + uint256 pulledHarb = 1000 ether; + uint256 discoveryAmount = 500 ether; + + // Must not revert — int128 guard skips mirror tick + strategy.setFloorPosition(CURRENT_TICK, floorEthBalance, pulledHarb, discoveryAmount, params); + + MockThreePositionStrategy.MintedPosition memory pos = strategy.getMintedPosition(0); + assertTrue(pos.liquidity > 0, "Floor should have liquidity even with extreme VWAP"); + } + + /// @notice Normal VWAP values (below int128 threshold) still compute mirror tick correctly. + function testFloorPositionNormalVWAPStillUsesMirror() public { + ThreePositionStrategy.PositionParams memory params = getDefaultParams(); + params.capitalInefficiency = 0; + + // Set a normal VWAP (1.0 in X96 format) — well below int128.max after >> 32 + uint256 normalVwap = 79_228_162_514_264_337_593_543_950_336; // 1.0 in X96 + strategy.setVWAP(normalVwap, 1000 ether); + + uint256 floorEthBalance = 80 ether; + uint256 pulledHarb = 1000 ether; + uint256 discoveryAmount = 500 ether; + + strategy.setFloorPosition(CURRENT_TICK, floorEthBalance, pulledHarb, discoveryAmount, params); + + MockThreePositionStrategy.MintedPosition memory pos = strategy.getMintedPosition(0); + assertEq(uint256(pos.stage), uint256(ThreePositionStrategy.Stage.FLOOR), "Should be floor position"); + assertTrue(pos.liquidity > 0, "Floor should have liquidity"); + } }