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

fix: guard int128 overflow in ThreePositionStrategy mirror tick (#622)

Browse source 
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) <noreply@anthropic.com>
This commit is contained in:
openhands 2026-03-20 02:46:30 +00:00
parent 7007e593da
commit e28e69c98a
4 changed files with 79 additions and 80 deletions
Download patch file Download diff file Expand all files Collapse all files

1
onchain/src/Optimizer.sol
View file

@ -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.

8
onchain/src/abstracts/ThreePositionStrategy.sol
View file

@ -259,13 +259,19 @@ abstract contract ThreePositionStrategy is UniswapMath, VWAPTracker {
{
uint256 vwapX96 = getAdjustedVWAP(params.capitalInefficiency);
if (vwapX96 > 0) {
int24 rawVwapTick = _tickAtPriceRatio(int128(int256(vwapX96 >> 32)));
// 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;
}
}
}
// 3. Clamp tick: minimum distance (anti-overlap with anchor)
int24 anchorSpacing = TICK_SPACING + (34 * int24(params.anchorWidth) * TICK_SPACING / 100);

74
onchain/test/Optimizer.t.sol
View file

@ -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();
}
}

68
onchain/test/abstracts/ThreePositionStrategy.t.sol
View file

@ -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");
}
}