From bdc17645f954070799fb4a1b53dc79b90a1a65ce Mon Sep 17 00:00:00 2001 From: johba Date: Sun, 22 Mar 2026 14:31:49 +0000 Subject: [PATCH] fix: Catch block skips clamping that try block applies (#1019) Add defence-in-depth assert statements in recenter()'s catch block to verify bear-mode constants (CI=0, AS=30%, AW=100, DD=0.3e18) satisfy the same bounds the try-path clamps to (MAX_PARAM_SCALE, MAX_ANCHOR_WIDTH). Add test verifying bear defaults are within clamping bounds and that the catch path deploys all three positions (floor, anchor, discovery). Co-Authored-By: Claude Opus 4.6 (1M context) --- onchain/src/LiquidityManager.sol | 11 +++++++++- onchain/test/LiquidityManager.t.sol | 34 +++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/onchain/src/LiquidityManager.sol b/onchain/src/LiquidityManager.sol index 8da6bc1..7878157 100644 --- a/onchain/src/LiquidityManager.sol +++ b/onchain/src/LiquidityManager.sol @@ -249,7 +249,16 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle { _setPositions(currentTick, params); } catch { - // Fallback to safe bear-mode defaults if optimizer fails + // Fallback to safe bear-mode defaults if optimizer fails. + // Defence-in-depth: assert bear constants satisfy the same bounds + // that the try-path clamps to. These are compile-time constants so + // the asserts can never fire at runtime, but they guard against a + // future edit to IOptimizer.sol that pushes a value out of range. + assert(BEAR_CAPITAL_INEFFICIENCY <= MAX_PARAM_SCALE); + assert(BEAR_ANCHOR_SHARE <= MAX_PARAM_SCALE); + assert(BEAR_ANCHOR_WIDTH <= MAX_ANCHOR_WIDTH); + assert(BEAR_DISCOVERY_DEPTH <= MAX_PARAM_SCALE); + PositionParams memory defaultParams = PositionParams({ capitalInefficiency: BEAR_CAPITAL_INEFFICIENCY, anchorShare: BEAR_ANCHOR_SHARE, diff --git a/onchain/test/LiquidityManager.t.sol b/onchain/test/LiquidityManager.t.sol index b3b6149..1444349 100644 --- a/onchain/test/LiquidityManager.t.sol +++ b/onchain/test/LiquidityManager.t.sol @@ -28,6 +28,7 @@ import { ThreePositionStrategy } from "../src/abstracts/ThreePositionStrategy.so import "../src/helpers/UniswapHelpers.sol"; import "../test/mocks/MockOptimizer.sol"; import "../test/mocks/ConfigurableOptimizer.sol"; +import { BEAR_CAPITAL_INEFFICIENCY, BEAR_ANCHOR_SHARE, BEAR_ANCHOR_WIDTH, BEAR_DISCOVERY_DEPTH } from "../src/IOptimizer.sol"; import { TestEnvironment } from "./helpers/TestBase.sol"; import { UniSwapHelper } from "./helpers/UniswapTestBase.sol"; @@ -1100,6 +1101,39 @@ contract LiquidityManagerTest is UniSwapHelper { assertGt(anchorLiq, 0, "ANCHOR position should have been created with fallback params"); } + /** + * @notice Catch-path clamping: verify bear defaults satisfy the same bounds + * that the try-path clamps to, and that all three positions are deployed. + * Covers issue #1019 — defence-in-depth assertions in catch block. + */ + function testOptimizerFallback_BearDefaultsWithinClampingBounds() public { + // --- Static check: bear constants are within clamping ceilings --- + uint256 MAX_PARAM_SCALE = 10 ** 18; + uint24 MAX_ANCHOR_WIDTH = 1233; + + assertLe(BEAR_CAPITAL_INEFFICIENCY, MAX_PARAM_SCALE, "BEAR_CAPITAL_INEFFICIENCY exceeds MAX_PARAM_SCALE"); + assertLe(BEAR_ANCHOR_SHARE, MAX_PARAM_SCALE, "BEAR_ANCHOR_SHARE exceeds MAX_PARAM_SCALE"); + assertLe(BEAR_ANCHOR_WIDTH, MAX_ANCHOR_WIDTH, "BEAR_ANCHOR_WIDTH exceeds MAX_ANCHOR_WIDTH"); + assertLe(BEAR_DISCOVERY_DEPTH, MAX_PARAM_SCALE, "BEAR_DISCOVERY_DEPTH exceeds MAX_PARAM_SCALE"); + + // --- Runtime check: catch block deploys all three positions --- + RevertingOptimizer revertingOpt = new RevertingOptimizer(); + TestEnvironment env = new TestEnvironment(feeDestination); + (,,,,, LiquidityManager _lm,,) = env.setupEnvironmentWithOptimizer(DEFAULT_TOKEN0_IS_WETH, address(revertingOpt)); + + vm.prank(RECENTER_CALLER); + _lm.recenter(); + + // All three position stages must have non-zero liquidity + (uint128 floorLiq,,) = _lm.positions(ThreePositionStrategy.Stage.FLOOR); + (uint128 anchorLiq,,) = _lm.positions(ThreePositionStrategy.Stage.ANCHOR); + (uint128 discoveryLiq,,) = _lm.positions(ThreePositionStrategy.Stage.DISCOVERY); + + assertGt(floorLiq, 0, "FLOOR position missing in catch path"); + assertGt(anchorLiq, 0, "ANCHOR position missing in catch path"); + assertGt(discoveryLiq, 0, "DISCOVERY position missing in catch path"); + } + /** * @notice When feeDestination == address(lm), recenter must not transfer fees externally * and _getOutstandingSupply must not double-subtract LM-held KRK.