From 42b4bf4149bdea40b747eb484b529e3981d252ea Mon Sep 17 00:00:00 2001 From: openhands Date: Fri, 20 Mar 2026 11:42:50 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20Shift=20field=20silently=20ignored?= =?UTF-8?q?=20=E2=80=94=20dyadic=20rational=20inputs=20effectively=20unsup?= =?UTF-8?q?ported=20(#606)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add require(shift == 0) guards to Optimizer.calculateParams and OptimizerV3.calculateParams so non-zero shifts revert instead of being silently discarded. OptimizerV3Push3 already had this guard. Update IOptimizer.sol NatSpec to document that shift is reserved for future use and must be 0 in all current implementations. Co-Authored-By: Claude Opus 4.6 (1M context) --- onchain/src/IOptimizer.sol | 5 ++++- onchain/src/Optimizer.sol | 4 +++- onchain/src/OptimizerV3.sol | 5 +++++ onchain/test/Optimizer.t.sol | 12 ++++++++++++ onchain/test/OptimizerV3Push3.t.sol | 11 +++++++++++ 5 files changed, 35 insertions(+), 2 deletions(-) diff --git a/onchain/src/IOptimizer.sol b/onchain/src/IOptimizer.sol index 047bcdd..0a4d537 100644 --- a/onchain/src/IOptimizer.sol +++ b/onchain/src/IOptimizer.sol @@ -3,7 +3,10 @@ pragma solidity ^0.8.19; /** * @notice Dyadic rational input: mantissa × 2^(-shift). - * For shift == 0 (current usage via _toDyadic), value == mantissa. + * shift is reserved for future use and MUST be 0. All current + * Optimizer implementations (Optimizer, OptimizerV3, OptimizerV3Push3) + * require shift == 0 and revert otherwise. When shift == 0 (as + * produced by _toDyadic), value == mantissa. */ struct OptimizerInput { int256 mantissa; diff --git a/onchain/src/Optimizer.sol b/onchain/src/Optimizer.sol index 7c596e7..e520456 100644 --- a/onchain/src/Optimizer.sol +++ b/onchain/src/Optimizer.sol @@ -356,8 +356,10 @@ contract Optimizer is Initializable, UUPSUpgradeable, IOptimizer { virtual returns (uint256 capitalInefficiency, uint256 anchorShare, uint24 anchorWidth, uint256 discoveryDepth) { - // Guard against negative mantissa — uint256() cast silently wraps negatives. + // Guard against non-zero shift and negative mantissa. + // shift is reserved for future use; uint256() cast silently wraps negatives. for (uint256 k; k < 8; k++) { + require(inputs[k].shift == 0, "shift not yet supported"); require(inputs[k].mantissa >= 0, "negative mantissa"); } // Extract slots 0 and 1 (shift=0 assumed — mantissa IS the value) diff --git a/onchain/src/OptimizerV3.sol b/onchain/src/OptimizerV3.sol index 8fa995e..ecfc30e 100644 --- a/onchain/src/OptimizerV3.sol +++ b/onchain/src/OptimizerV3.sol @@ -22,6 +22,11 @@ contract OptimizerV3 is Optimizer { override returns (uint256 ci, uint256 anchorShare, uint24 anchorWidth, uint256 discoveryDepth) { + // Guard against non-zero shift (reserved for future use). + for (uint256 k; k < 8; k++) { + require(inputs[k].shift == 0, "shift not yet supported"); + } + // ── BEGIN TRANSPILER OUTPUT (optimizer_v3.push3) ── // Do NOT edit by hand — regenerate via: npx tsx tools/push3-transpiler/transpile-cli.ts diff --git a/onchain/test/Optimizer.t.sol b/onchain/test/Optimizer.t.sol index 18e0728..57daa62 100644 --- a/onchain/test/Optimizer.t.sol +++ b/onchain/test/Optimizer.t.sol @@ -359,6 +359,18 @@ contract OptimizerTest is Test { } } + /** + * @notice calculateParams reverts when any slot has shift != 0 + */ + function testCalculateParamsRevertsOnNonZeroShift() public { + for (uint256 k = 0; k < 8; k++) { + OptimizerInput[8] memory inputs; + inputs[k] = OptimizerInput({ mantissa: 0, shift: 1 }); + vm.expectRevert("shift not yet supported"); + optimizer.calculateParams(inputs); + } + } + /** * @notice Non-admin calling upgradeTo should revert with UnauthorizedAccount */ diff --git a/onchain/test/OptimizerV3Push3.t.sol b/onchain/test/OptimizerV3Push3.t.sol index bae7169..f606c41 100644 --- a/onchain/test/OptimizerV3Push3.t.sol +++ b/onchain/test/OptimizerV3Push3.t.sol @@ -224,6 +224,17 @@ contract OptimizerV3Push3Test is Test { push3.calculateParams(inp); } + // ---- Shift guard ---- + + function testNonZeroShiftReverts() public { + for (uint256 k = 0; k < 8; k++) { + OptimizerInput[8] memory inp; + inp[k] = OptimizerInput({mantissa: 0, shift: 1}); + vm.expectRevert("shift not yet supported"); + push3.calculateParams(inp); + } + } + // ---- Fuzz ---- function testFuzzNeverReverts(uint256 percentageStaked, uint256 averageTaxRate) public view { From abbf14854df1bf6c05490a7ebcb6798893f597d7 Mon Sep 17 00:00:00 2001 From: openhands Date: Fri, 20 Mar 2026 12:10:03 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20add?= =?UTF-8?q?=20negative-mantissa=20guard=20to=20OptimizerV3,=20add=20Optimi?= =?UTF-8?q?zerV3=20test=20file?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add require(mantissa >= 0) to OptimizerV3.calculateParams validation loop (was missing unlike Optimizer and OptimizerV3Push3) - Create onchain/test/OptimizerV3.t.sol with shift, negative-mantissa, and basic bear/bull output tests - Fix stale comment in Optimizer.sol: "shift=0 assumed" → "shift=0 enforced" - Use implementation-neutral NatSpec phrasing in IOptimizer.sol Co-Authored-By: Claude Opus 4.6 (1M context) --- onchain/src/IOptimizer.sol | 7 ++- onchain/src/Optimizer.sol | 2 +- onchain/src/OptimizerV3.sol | 4 +- onchain/test/OptimizerV3.t.sol | 85 ++++++++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 6 deletions(-) create mode 100644 onchain/test/OptimizerV3.t.sol diff --git a/onchain/src/IOptimizer.sol b/onchain/src/IOptimizer.sol index 0a4d537..4531a8a 100644 --- a/onchain/src/IOptimizer.sol +++ b/onchain/src/IOptimizer.sol @@ -3,10 +3,9 @@ pragma solidity ^0.8.19; /** * @notice Dyadic rational input: mantissa × 2^(-shift). - * shift is reserved for future use and MUST be 0. All current - * Optimizer implementations (Optimizer, OptimizerV3, OptimizerV3Push3) - * require shift == 0 and revert otherwise. When shift == 0 (as - * produced by _toDyadic), value == mantissa. + * shift is reserved for future use and MUST be 0. All production + * Optimizer implementations require shift == 0 and revert otherwise. + * When shift == 0 (as produced by _toDyadic), value == mantissa. */ struct OptimizerInput { int256 mantissa; diff --git a/onchain/src/Optimizer.sol b/onchain/src/Optimizer.sol index e520456..e08d835 100644 --- a/onchain/src/Optimizer.sol +++ b/onchain/src/Optimizer.sol @@ -362,7 +362,7 @@ contract Optimizer is Initializable, UUPSUpgradeable, IOptimizer { require(inputs[k].shift == 0, "shift not yet supported"); require(inputs[k].mantissa >= 0, "negative mantissa"); } - // Extract slots 0 and 1 (shift=0 assumed — mantissa IS the value) + // Extract slots 0 and 1 (shift=0 enforced above — mantissa IS the value) uint256 percentageStaked = uint256(inputs[0].mantissa); uint256 averageTaxRate = uint256(inputs[1].mantissa); diff --git a/onchain/src/OptimizerV3.sol b/onchain/src/OptimizerV3.sol index ecfc30e..e7375f9 100644 --- a/onchain/src/OptimizerV3.sol +++ b/onchain/src/OptimizerV3.sol @@ -22,9 +22,11 @@ contract OptimizerV3 is Optimizer { override returns (uint256 ci, uint256 anchorShare, uint24 anchorWidth, uint256 discoveryDepth) { - // Guard against non-zero shift (reserved for future use). + // Guard against non-zero shift and negative mantissa. + // shift is reserved for future use; uint256() cast silently wraps negatives. for (uint256 k; k < 8; k++) { require(inputs[k].shift == 0, "shift not yet supported"); + require(inputs[k].mantissa >= 0, "negative mantissa"); } // ── BEGIN TRANSPILER OUTPUT (optimizer_v3.push3) ── diff --git a/onchain/test/OptimizerV3.t.sol b/onchain/test/OptimizerV3.t.sol new file mode 100644 index 0000000..fc3b662 --- /dev/null +++ b/onchain/test/OptimizerV3.t.sol @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.19; + +import {OptimizerV3} from "../src/OptimizerV3.sol"; +import {OptimizerInput} from "../src/IOptimizer.sol"; +import "forge-std/Test.sol"; + +/** + * @title OptimizerV3Test + * @notice Unit tests for OptimizerV3.calculateParams input validation guards + * (shift and negative-mantissa) and basic bear/bull output correctness. + */ +contract OptimizerV3Test is Test { + OptimizerV3 v3; + + function setUp() public { + v3 = new OptimizerV3(); + } + + // ---- Helpers ---- + + function _inputs(uint256 percentageStaked, uint256 averageTaxRate) + internal + pure + returns (OptimizerInput[8] memory inp) + { + inp[0] = OptimizerInput({mantissa: int256(percentageStaked), shift: 0}); + inp[1] = OptimizerInput({mantissa: int256(averageTaxRate), shift: 0}); + } + + // ---- Shift guard ---- + + function testNonZeroShiftReverts() public { + for (uint256 k = 0; k < 8; k++) { + OptimizerInput[8] memory inp; + inp[k] = OptimizerInput({mantissa: 0, shift: 1}); + vm.expectRevert("shift not yet supported"); + v3.calculateParams(inp); + } + } + + // ---- Negative mantissa guard ---- + + function testNegativeMantissaSlot0Reverts() public { + OptimizerInput[8] memory inp; + inp[0] = OptimizerInput({mantissa: -1, shift: 0}); + vm.expectRevert("negative mantissa"); + v3.calculateParams(inp); + } + + function testNegativeMantissaSlot1Reverts() public { + OptimizerInput[8] memory inp; + inp[0] = OptimizerInput({mantissa: int256(95e16), shift: 0}); + inp[1] = OptimizerInput({mantissa: -1, shift: 0}); + vm.expectRevert("negative mantissa"); + v3.calculateParams(inp); + } + + function testNegativeMantissaHighSlotReverts() public { + OptimizerInput[8] memory inp; + inp[5] = OptimizerInput({mantissa: -1, shift: 0}); + vm.expectRevert("negative mantissa"); + v3.calculateParams(inp); + } + + // ---- Basic output correctness ---- + + function testBearAtLowStaking() public view { + (uint256 ci, uint256 as_, uint24 aw, uint256 dd) = + v3.calculateParams(_inputs(50e16, 0)); + assertEq(ci, 0); + assertEq(as_, 3e17); + assertEq(aw, 100); + assertEq(dd, 3e17); + } + + function testBullAtHighStaking() public view { + (uint256 ci, uint256 as_, uint24 aw, uint256 dd) = + v3.calculateParams(_inputs(96e16, 0)); + assertEq(ci, 0); + assertEq(as_, 1e18); + assertEq(aw, 20); + assertEq(dd, 1e18); + } +}