fix: address review — add negative-mantissa guard to OptimizerV3, add OptimizerV3 test file

- 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) <noreply@anthropic.com>
This commit is contained in:
openhands 2026-03-20 12:10:03 +00:00
parent 42b4bf4149
commit abbf14854d
4 changed files with 92 additions and 6 deletions

View file

@ -3,10 +3,9 @@ pragma solidity ^0.8.19;
/** /**
* @notice Dyadic rational input: mantissa × 2^(-shift). * @notice Dyadic rational input: mantissa × 2^(-shift).
* shift is reserved for future use and MUST be 0. All current * shift is reserved for future use and MUST be 0. All production
* Optimizer implementations (Optimizer, OptimizerV3, OptimizerV3Push3) * Optimizer implementations require shift == 0 and revert otherwise.
* require shift == 0 and revert otherwise. When shift == 0 (as * When shift == 0 (as produced by _toDyadic), value == mantissa.
* produced by _toDyadic), value == mantissa.
*/ */
struct OptimizerInput { struct OptimizerInput {
int256 mantissa; int256 mantissa;

View file

@ -362,7 +362,7 @@ contract Optimizer is Initializable, UUPSUpgradeable, IOptimizer {
require(inputs[k].shift == 0, "shift not yet supported"); require(inputs[k].shift == 0, "shift not yet supported");
require(inputs[k].mantissa >= 0, "negative mantissa"); 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 percentageStaked = uint256(inputs[0].mantissa);
uint256 averageTaxRate = uint256(inputs[1].mantissa); uint256 averageTaxRate = uint256(inputs[1].mantissa);

View file

@ -22,9 +22,11 @@ contract OptimizerV3 is Optimizer {
override override
returns (uint256 ci, uint256 anchorShare, uint24 anchorWidth, uint256 discoveryDepth) 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++) { for (uint256 k; k < 8; k++) {
require(inputs[k].shift == 0, "shift not yet supported"); require(inputs[k].shift == 0, "shift not yet supported");
require(inputs[k].mantissa >= 0, "negative mantissa");
} }
// BEGIN TRANSPILER OUTPUT (optimizer_v3.push3) // BEGIN TRANSPILER OUTPUT (optimizer_v3.push3)

View file

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