Merge pull request 'fix: Test coverage: LiquidityManager + Optimizer + OptimizerV3 to 100% (#285)' (#296) from fix/issue-285 into master

This commit is contained in:
johba 2026-02-26 04:07:11 +01:00
commit a7947c64c8
4 changed files with 376 additions and 0 deletions

View file

@ -3,3 +3,4 @@
@uniswap-v3-periphery/=lib/uni-v3-lib/node_modules/@uniswap/v3-periphery/contracts/
@aperture/uni-v3-lib/=lib/uni-v3-lib/src/
@abdk/=lib/abdk-libraries-solidity/
solady/src/=lib/uni-v3-lib/node_modules/solady/src/

View file

@ -62,6 +62,30 @@ contract Dummy {
// This contract can be empty as it is only used to affect the nonce
}
/// @notice Harness that exposes LiquidityManager's internal abstract functions for coverage
contract LiquidityManagerHarness is LiquidityManager {
constructor(address _factory, address _WETH9, address _kraiken, address _optimizer)
LiquidityManager(_factory, _WETH9, _kraiken, _optimizer)
{ }
function exposed_getKraikenToken() external view returns (address) {
return _getKraikenToken();
}
function exposed_getWethToken() external view returns (address) {
return _getWethToken();
}
}
/// @notice Optimizer that always reverts getLiquidityParams triggers the catch block in LiquidityManager
contract RevertingOptimizer {
function getLiquidityParams() external pure returns (uint256, uint256, uint24, uint256) {
revert("optimizer failed");
}
function initialize(address, address) external { }
}
contract LiquidityManagerTest is UniSwapHelper {
// Constant address for recenter operations
address constant RECENTER_CALLER = address(0x7777);
@ -922,4 +946,144 @@ contract LiquidityManagerTest is UniSwapHelper {
console.log("PASS: Asymmetric liquidity profile maintained");
console.log("PASS: Attack protection mechanism validated");
}
// =========================================================
// COVERAGE TESTS: onlyFeeDestination, revokeRecenterAccess,
// open recenter path, VWAP else branch,
// optimizer fallback, _getKraikenToken/_getWethToken
// =========================================================
/**
* @notice Calling an onlyFeeDestination function from a non-fee address must revert
*/
function testOnlyFeeDestinationReverts() public {
address nonFee = makeAddr("notFeeDestination");
vm.expectRevert("only callable by feeDestination");
vm.prank(nonFee);
lm.setRecenterAccess(nonFee);
}
/**
* @notice feeDestination can revoke recenter access (covers revokeRecenterAccess body)
*/
function testRevokeRecenterAccess() public {
assertEq(lm.recenterAccess(), RECENTER_CALLER, "precondition: access should be set");
vm.prank(feeDestination);
lm.revokeRecenterAccess();
assertEq(lm.recenterAccess(), address(0), "recenterAccess should be revoked");
}
/**
* @notice Open recenter (no access restriction) must fail with cooldown if called too soon
*/
function testOpenRecenterCooldown() public {
vm.prank(feeDestination);
lm.revokeRecenterAccess();
// Immediately try to recenter without waiting should hit cooldown check
vm.expectRevert("recenter cooldown");
lm.recenter();
}
/**
* @notice After cooldown, open recenter calls _isPriceStable (covering _getPool) then
* hits amplitude check (covers the open-recenter else branch, lines 141-142, 265-266)
* @dev PriceOracle._isPriceStable has a 60,000-second fallback interval.
* setUp warps ~18,000s so the pool's history is only ~18,000s.
* We warp an additional 61,000s so pool history > 60,000s for the fallback to succeed.
*/
function testOpenRecenterOracleCheck() public {
vm.prank(feeDestination);
lm.revokeRecenterAccess();
// Warp enough seconds so pool.observe([300,0]) and its fallback ([60000,0]) both succeed.
// Pool was initialized at timestamp 1; after setUp + this warp: ~79,001s of history.
vm.warp(block.timestamp + 61_000);
// _isPriceStable ( _getPool) is called; price unchanged stable.
// Amplitude check fires because price hasn't moved since last recenter.
vm.expectRevert("amplitude not reached.");
lm.recenter();
}
/**
* @notice ZeroAddressInSetter revert when setting fee destination to address(0)
*/
function testSetFeeDestinationZeroAddress() public {
// Deploy a fresh LM with this test contract as deployer
LiquidityManager freshLm = new LiquidityManager(address(factory), address(weth), address(harberg), address(optimizer));
vm.expectRevert(LiquidityManager.ZeroAddressInSetter.selector);
freshLm.setFeeDestination(address(0));
}
/**
* @notice AddressAlreadySet revert when fee destination is changed after initial set
*/
function testSetFeeDestinationAlreadySet() public {
// Deploy a fresh LM with this test contract as deployer
LiquidityManager freshLm = new LiquidityManager(address(factory), address(weth), address(harberg), address(optimizer));
freshLm.setFeeDestination(makeAddr("firstFee"));
vm.expectRevert(LiquidityManager.AddressAlreadySet.selector);
freshLm.setFeeDestination(makeAddr("secondFee"));
}
/**
* @notice When optimizer reverts, the catch block uses default bear params (covers lines 192-201)
*/
function testOptimizerFallback() public {
RevertingOptimizer revertingOpt = new RevertingOptimizer();
TestEnvironment env = new TestEnvironment(feeDestination);
(,,,,, LiquidityManager _lm,,) = env.setupEnvironmentWithOptimizer(DEFAULT_TOKEN0_IS_WETH, RECENTER_CALLER, address(revertingOpt));
// Recenter uses the fallback params from the catch block
vm.prank(RECENTER_CALLER);
bool isUp = _lm.recenter();
// First recenter: no previous anchor so amplitude check is skipped, isUp stays false
assertFalse(isUp, "First recenter should return false");
// Verify positions were created with the fallback (bear) params ANCHOR must exist
(uint128 anchorLiq,,) = _lm.positions(ThreePositionStrategy.Stage.ANCHOR);
assertGt(anchorLiq, 0, "ANCHOR position should have been created with fallback params");
}
/**
* @notice VWAP else branch: cumulativeVolume > 0 AND lastRecenterTick == 0
* Reached by re-deploying a fresh LM (lastRecenterTick = 0) and setting
* cumulativeVolume via vm.store before the first recenter.
*/
function testVWAPElseBranch() public {
// Re-deploy fresh protocol so lastRecenterTick starts at 0
deployProtocolWithTokenOrder(DEFAULT_TOKEN0_IS_WETH);
// Preconditions: fresh LM has lastRecenterTick == 0 and cumulativeVolume == 0
assertEq(lm.lastRecenterTick(), 0, "precondition: lastRecenterTick must be 0");
assertEq(lm.cumulativeVolume(), 0, "precondition: cumulativeVolume must be 0");
// Set cumulativeVolume to non-zero (storage slot 1 in VWAPTracker).
// This creates the state: cumulativeVolume > 0 AND lastRecenterTick == 0,
// which triggers the else branch (line 170) on the first recenter.
vm.store(address(lm), bytes32(uint256(1)), bytes32(uint256(1e18)));
assertEq(lm.cumulativeVolume(), 1e18, "cumulativeVolume should be 1e18 after vm.store");
// Call recenter hits the else branch, sets lastRecenterTick, then sets positions
vm.prank(RECENTER_CALLER);
lm.recenter();
// Verify lastRecenterTick was updated from the recenter
assertTrue(lm.lastRecenterTick() != 0, "lastRecenterTick should have been updated after recenter");
}
/**
* @notice Deploy a LiquidityManagerHarness and call the exposed abstract functions
* to cover _getKraikenToken() and _getWethToken() (lines 270-271, 275-276)
*/
function testHarnessAbstractFunctions() public {
LiquidityManagerHarness harness =
new LiquidityManagerHarness(address(factory), address(weth), address(harberg), address(optimizer));
assertEq(harness.exposed_getKraikenToken(), address(harberg), "_getKraikenToken should return kraiken");
assertEq(harness.exposed_getWethToken(), address(weth), "_getWethToken should return weth");
}
}

View file

@ -7,6 +7,14 @@ import "./mocks/MockKraiken.sol";
import "./mocks/MockStake.sol";
import "forge-std/Test.sol";
import "forge-std/console.sol";
import { ERC1967Proxy } from "@openzeppelin/proxy/ERC1967/ERC1967Proxy.sol";
/// @dev Harness to expose internal _calculateAnchorWidth for direct coverage of the totalWidth < 10 path
contract OptimizerHarness is Optimizer {
function exposed_calculateAnchorWidth(uint256 percentageStaked, uint256 averageTaxRate) external pure returns (uint24) {
return _calculateAnchorWidth(percentageStaked, averageTaxRate);
}
}
contract OptimizerTest is Test {
Optimizer optimizer;
@ -236,4 +244,104 @@ contract OptimizerTest is Test {
// Expected: base(40) + staking_adj(20 - 24 = -4) + tax_adj(16 - 10 = 6) = 42
assertEq(anchorWidth, 42, "Anchor width should be independently calculated");
}
// =========================================================
// COVERAGE TESTS: calculateSentiment direct call + mid-range tax + zero path
// =========================================================
/**
* @notice Direct external call to calculateSentiment covers the function in coverage metrics
*/
function testCalculateSentimentDirect() public view {
// 100% staked, any tax high staking path very low penalty
uint256 sentiment = optimizer.calculateSentiment(0, 1e18);
// deltaS = 0, penalty = 0, sentimentValue = 0
assertEq(sentiment, 0, "100% staked, 0 tax: penalty=0 so sentiment=0");
}
/**
* @notice Cover the else-if (averageTaxRate <= 5e16) branch with a result > 0
* @dev averageTaxRate = 3e16 (in range (1e16, 5e16]), percentageStaked = 0
* baseSentiment = 1e18, ratePenalty = (2e16 * 1e18) / 4e16 = 5e17
* result = 1e18 - 5e17 = 5e17
*/
function testCalculateSentimentMidRangeTax() public view {
uint256 sentiment = optimizer.calculateSentiment(3e16, 0);
assertEq(sentiment, 5e17, "Mid-range tax should apply partial penalty");
}
/**
* @notice Cover the ternary zero path: baseSentiment > ratePenalty ? ... : 0
* @dev averageTaxRate = 5e16 (boundary), percentageStaked = 0
* baseSentiment = 1e18, ratePenalty = (4e16 * 1e18) / 4e16 = 1e18
* 1e18 > 1e18 is false sentimentValue = 0
*/
function testCalculateSentimentZeroPath() public view {
uint256 sentiment = optimizer.calculateSentiment(5e16, 0);
assertEq(sentiment, 0, "At boundary 5e16 ratePenalty equals baseSentiment so result is zero");
}
// =========================================================
// COVERAGE TESTS: UUPS upgrade flow (_checkAdmin, _authorizeUpgrade, onlyAdmin)
// =========================================================
/**
* @notice Deploy via ERC1967Proxy and call upgradeTo to cover _authorizeUpgrade + _checkAdmin
*/
function testUUPSUpgrade() public {
Optimizer impl1 = new Optimizer();
ERC1967Proxy proxy = new ERC1967Proxy(
address(impl1),
abi.encodeWithSelector(Optimizer.initialize.selector, address(mockKraiken), address(mockStake))
);
Optimizer proxyOptimizer = Optimizer(address(proxy));
// Deployer (this contract) is admin upgrade should succeed
Optimizer impl2 = new Optimizer();
proxyOptimizer.upgradeTo(address(impl2));
// Verify proxy still works after upgrade
(,, uint24 w,) = proxyOptimizer.getLiquidityParams();
assertTrue(w >= 10 && w <= 80, "Params should still work after upgrade");
}
/**
* @notice Cover the require revert branch in calculateSentiment (percentageStaked > 1e18)
*/
function testCalculateSentimentRevertsAbove100Percent() public {
vm.expectRevert("Invalid percentage staked");
optimizer.calculateSentiment(0, 1e18 + 1);
}
/**
* @notice Cover the totalWidth < 10 clamp via OptimizerHarness.
* @dev With percentageStaked = 1.5e18 and averageTaxRate = 0:
* stakingAdjustment = 20 - 60 = -40
* taxAdjustment = 0 - 10 = -10
* totalWidth = 40 - 40 - 10 = -10 clamped to 10
*/
function testAnchorWidthBelowTenClamp() public {
OptimizerHarness harness = new OptimizerHarness();
uint24 w = harness.exposed_calculateAnchorWidth(15e17, 0);
assertEq(w, 10, "totalWidth < 10 should be clamped to minimum of 10");
}
/**
* @notice Non-admin calling upgradeTo should revert with UnauthorizedAccount
*/
function testUnauthorizedUpgradeReverts() public {
Optimizer impl1 = new Optimizer();
ERC1967Proxy proxy = new ERC1967Proxy(
address(impl1),
abi.encodeWithSelector(Optimizer.initialize.selector, address(mockKraiken), address(mockStake))
);
Optimizer proxyOptimizer = Optimizer(address(proxy));
// Deploy impl2 BEFORE the prank so the prank applies only to upgradeTo
Optimizer impl2 = new Optimizer();
address nonAdmin = makeAddr("nonAdmin");
vm.expectRevert(abi.encodeWithSelector(Optimizer.UnauthorizedAccount.selector, nonAdmin));
vm.prank(nonAdmin);
proxyOptimizer.upgradeTo(address(impl2));
}
}

View file

@ -4,6 +4,9 @@ pragma solidity ^0.8.19;
import { OptimizerV3 } from "../src/OptimizerV3.sol";
import { Stake } from "../src/Stake.sol";
import "forge-std/Test.sol";
import "./mocks/MockKraiken.sol";
import "./mocks/MockStake.sol";
import { ERC1967Proxy } from "@openzeppelin/proxy/ERC1967/ERC1967Proxy.sol";
contract OptimizerV3Test is Test {
OptimizerV3 optimizer;
@ -230,3 +233,103 @@ contract OptimizerV3Test is Test {
optimizer.isBullMarket(percentageStaked, averageTaxRate);
}
}
// =========================================================
// COVERAGE TESTS: initialize, getLiquidityParams, UUPS upgrade
// =========================================================
/**
* @title OptimizerV3ProxyTest
* @notice Tests OptimizerV3 features that require proxy deployment:
* initialize, getLiquidityParams, _authorizeUpgrade, onlyAdmin, _checkAdmin
*/
contract OptimizerV3ProxyTest is Test {
MockKraiken mockKraiken;
MockStake mockStake;
OptimizerV3 proxyOptimizer;
function setUp() public {
mockKraiken = new MockKraiken();
mockStake = new MockStake();
OptimizerV3 impl = new OptimizerV3();
ERC1967Proxy proxy = new ERC1967Proxy(
address(impl),
abi.encodeWithSelector(OptimizerV3.initialize.selector, address(mockKraiken), address(mockStake))
);
proxyOptimizer = OptimizerV3(address(proxy));
}
/**
* @notice Verify initialize set up the proxy correctly (covers initialize body)
*/
function testInitialize() public view {
// The proxy was initialized calling getLiquidityParams should not revert
// (it calls stake.getPercentageStaked() and stake.getAverageTaxRate())
(uint256 ci, uint256 as_, uint24 aw, uint256 dd) = proxyOptimizer.getLiquidityParams();
assertEq(ci, 0, "capitalInefficiency always 0");
assertTrue(aw == 100 || aw == 20, "anchorWidth is either bear(100) or bull(20)");
// Bear or bull values are valid
assertTrue(
(as_ == 3e17 && dd == 3e17 && aw == 100) || (as_ == 1e18 && dd == 1e18 && aw == 20),
"Params should be valid bear or bull set"
);
}
/**
* @notice Bear market: staked <= 91% AS=30%, AW=100, DD=0.3e18, CI=0
*/
function testGetLiquidityParamsBear() public {
mockStake.setPercentageStaked(50e16); // 50% staked always bear
mockStake.setAverageTaxRate(0);
(uint256 capitalInefficiency, uint256 anchorShare, uint24 anchorWidth, uint256 discoveryDepth) =
proxyOptimizer.getLiquidityParams();
assertEq(capitalInefficiency, 0, "Bear: CI=0");
assertEq(anchorShare, 3e17, "Bear: AS=30%");
assertEq(anchorWidth, 100, "Bear: AW=100");
assertEq(discoveryDepth, 3e17, "Bear: DD=0.3e18");
}
/**
* @notice Bull market: staked > 91%, low tax AS=100%, AW=20, DD=1e18, CI=0
*/
function testGetLiquidityParamsBull() public {
mockStake.setPercentageStaked(1e18); // 100% staked always bull
mockStake.setAverageTaxRate(0);
(uint256 capitalInefficiency, uint256 anchorShare, uint24 anchorWidth, uint256 discoveryDepth) =
proxyOptimizer.getLiquidityParams();
assertEq(capitalInefficiency, 0, "Bull: CI=0");
assertEq(anchorShare, 1e18, "Bull: AS=100%");
assertEq(anchorWidth, 20, "Bull: AW=20");
assertEq(discoveryDepth, 1e18, "Bull: DD=1e18");
}
/**
* @notice Admin can upgrade to a new implementation (covers _authorizeUpgrade, onlyAdmin, _checkAdmin)
*/
function testV3UUPSUpgrade() public {
OptimizerV3 impl2 = new OptimizerV3();
// This contract (address(this)) is the admin since it deployed the proxy
proxyOptimizer.upgradeTo(address(impl2));
// Verify proxy still works after upgrade
(uint256 ci,,,) = proxyOptimizer.getLiquidityParams();
assertEq(ci, 0, "CI always 0 after upgrade");
}
/**
* @notice Non-admin calling upgradeTo reverts with UnauthorizedAccount
*/
function testV3UnauthorizedUpgradeReverts() public {
// Deploy impl2 BEFORE the prank so the prank applies only to upgradeTo
OptimizerV3 impl2 = new OptimizerV3();
address nonAdmin = makeAddr("nonAdmin");
vm.expectRevert(abi.encodeWithSelector(OptimizerV3.UnauthorizedAccount.selector, nonAdmin));
vm.prank(nonAdmin);
proxyOptimizer.upgradeTo(address(impl2));
}
}