From 28ce5ec8cd2d53caee85d971099cce942e49fd95 Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 18 Mar 2026 18:24:18 +0000 Subject: [PATCH] fix: Optimizer.sol base class only guards slots 0 and 1 (#968) Replace the two per-slot require checks with a loop over all 8 input slots so future subclasses using slots 2-7 are protected from silent uint256 wrap. Add testCalculateParamsRevertsOnNegativeMantissaSlots2to7 to verify the guard. Co-Authored-By: Claude Sonnet 4.6 --- onchain/src/Optimizer.sol | 5 +++-- onchain/test/Optimizer.t.sol | 12 ++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/onchain/src/Optimizer.sol b/onchain/src/Optimizer.sol index 954c775..6a502d3 100644 --- a/onchain/src/Optimizer.sol +++ b/onchain/src/Optimizer.sol @@ -370,8 +370,9 @@ contract Optimizer is Initializable, UUPSUpgradeable, IOptimizer { returns (uint256 capitalInefficiency, uint256 anchorShare, uint24 anchorWidth, uint256 discoveryDepth) { // Guard against negative mantissa — uint256() cast silently wraps negatives. - require(inputs[0].mantissa >= 0, "negative mantissa"); - require(inputs[1].mantissa >= 0, "negative mantissa"); + for (uint256 k; k < 8; k++) { + require(inputs[k].mantissa >= 0, "negative mantissa"); + } // Extract slots 0 and 1 (shift=0 assumed — mantissa IS the value) uint256 percentageStaked = uint256(inputs[0].mantissa); uint256 averageTaxRate = uint256(inputs[1].mantissa); diff --git a/onchain/test/Optimizer.t.sol b/onchain/test/Optimizer.t.sol index 99f177c..5bc5fcc 100644 --- a/onchain/test/Optimizer.t.sol +++ b/onchain/test/Optimizer.t.sol @@ -347,6 +347,18 @@ contract OptimizerTest is Test { optimizer.calculateParams(inputs); } + /** + * @notice calculateParams reverts when any of inputs[2..7].mantissa is negative + */ + function testCalculateParamsRevertsOnNegativeMantissaSlots2to7() public { + for (uint256 k = 2; k < 8; k++) { + OptimizerInput[8] memory inputs; + inputs[k] = OptimizerInput({mantissa: -1, shift: 0}); + vm.expectRevert("negative mantissa"); + optimizer.calculateParams(inputs); + } + } + /** * @notice Non-admin calling upgradeTo should revert with UnauthorizedAccount */