From 0435dca8e38ac61d90bd06856ad9634e7b6cc77c Mon Sep 17 00:00:00 2001 From: jayesh yadav Date: Sat, 18 Jul 2026 16:00:53 +0530 Subject: [PATCH] fix(H1/H2): high-water-mark performance fee + minimum term duration Audit H1/H2: the performance fee was charged on (nav - protectedAmount), i.e. NAV above the 90% floor, so a flat term (nav 100 vs floor 90) or a losing term (nav 95) still minted a 'performance' fee; and because every new term re-strikes the floor at 0.9x current nav with no minimum term, a keeper could loop settleTerm->startTerm to mint ~perfBps*(1-P) of NAV per cycle indefinitely. Fix: the performance fee is now charged only on the rise in navPerShare above a per-share high-water mark, which then ratchets up. Flat/losing terms and re-struck terms where navPerShare has not advanced pay zero, and gains are never double-charged across terms. HWM is seeded at the first startTerm's entry price. Also added a 7-day MIN_TERM_DURATION in the controller as anti-churn defense-in-depth. Composes with the H3 per-share design (deposits never move navPerShare, so the basis is clean). Regression tests: no fee on flat/losing terms, HWM holds across terms, min-term blocks 1-second churn. 116 -> 120. --- src/CPPIController.sol | 6 ++++ src/CPPIVault.sol | 27 +++++++++++--- test/VaultIntegration.t.sol | 70 ++++++++++++++++++++++++++++++++++--- 3 files changed, 93 insertions(+), 10 deletions(-) diff --git a/src/CPPIController.sol b/src/CPPIController.sol index 89f4ad1..bc1f4e4 100644 --- a/src/CPPIController.sol +++ b/src/CPPIController.sol @@ -38,6 +38,10 @@ contract CPPIController { uint256 internal constant MAX_RATE_STEP_WAD = 0.02e18; uint256 internal lastRateWad; + /// @dev Anti-churn floor on term length (audit H2). Well below the 12-month + /// product term; exists so a compromised keeper cannot loop tiny terms. + uint64 public constant MIN_TERM_DURATION = 7 days; + event TermStarted( uint64 indexed termNumber, uint64 termStart, uint64 termEnd, uint256 nav, uint256 protectedAmount ); @@ -49,6 +53,7 @@ contract CPPIController { error TermStillActive(); error TermNotMatured(); error ZeroSupply(); + error TermTooShort(); modifier onlyVault() { if (msg.sender != vault) revert NotVault(); @@ -78,6 +83,7 @@ contract CPPIController { function startTerm(uint64 termStart, uint64 termEnd, uint256 nav, uint256 supply) external onlyVault { if (termActive) revert TermStillActive(); if (supply == 0) revert ZeroSupply(); + if (termEnd < termStart + MIN_TERM_DURATION) revert TermTooShort(); floorConfig.termStart = termStart; floorConfig.termEnd = termEnd; FloorPolicy.validate(floorConfig); diff --git a/src/CPPIVault.sol b/src/CPPIVault.sol index 36da2d2..488600d 100644 --- a/src/CPPIVault.sol +++ b/src/CPPIVault.sol @@ -46,7 +46,12 @@ contract CPPIVault is ERC20, Ownable { // fees: caps enforced in code, zero by default (curator loss-leader norm) uint16 public managementFeeBps; // per year, on shareholder NAV - uint16 public performanceFeeBps; // on term gains above the protected amount + uint16 public performanceFeeBps; // on gains in navPerShare above the high-water mark + /// @dev Per-share high-water mark: the highest navPerShare a performance + /// fee has been charged at. The fee applies only to gains above it, + /// so flat/losing terms and re-struck short terms pay nothing (audit + /// H1/H2). Initialized at the first startTerm. + uint256 public highWaterPerShareWad; address public feeRecipient; uint64 public lastMgmtAccrualAt; uint16 public constant MAX_MANAGEMENT_FEE_BPS = 200; @@ -379,21 +384,33 @@ contract CPPIVault is ERC20, Ownable { // ---------- term lifecycle ---------- function startTerm(uint64 duration) external onlyKeeper { + // seed the high-water mark at the first term's entry navPerShare so the + // performance fee is measured from real principal, not from zero + if (highWaterPerShareWad == 0) highWaterPerShareWad = navPerShare(); controller.startTerm( uint64(block.timestamp), uint64(block.timestamp) + duration, shareholderNav(), totalSupply() ); } + /// @dev Performance fee is charged only on the rise in navPerShare above + /// the per-share high-water mark, then the mark ratchets up. Flat or + /// losing terms, and re-struck short terms where navPerShare has not + /// advanced, pay nothing (audit H1/H2). Deposits never move + /// navPerShare (they mint at price), so the per-share basis is clean. function settleTerm() external onlyKeeper returns (uint256 shortfall) { _accrueManagementFee(); uint256 supply = totalSupply(); - uint256 protectedAmount = controller.protectedAmount(supply); uint256 nav = shareholderNav(); shortfall = controller.settleTerm(nav, supply); - if (performanceFeeBps > 0 && feeRecipient != address(0) && nav > protectedAmount) { - uint256 gainWad = nav - protectedAmount; + if (performanceFeeBps == 0 || feeRecipient == address(0) || supply == 0) return shortfall; + + uint256 navPS = navPerShare(); + uint256 hwm = highWaterPerShareWad; + if (navPS > hwm) { + uint256 gainWad = (navPS - hwm).mulWad(supply); uint256 feeWad = gainWad * performanceFeeBps / 10_000; - uint256 feeShares = feeWad.divWad(navPerShare()); + uint256 feeShares = feeWad.divWad(navPS); + highWaterPerShareWad = navPS; // ratchet the mark up _mint(feeRecipient, feeShares); emit PerformanceFeeCharged(feeShares, gainWad); } diff --git a/test/VaultIntegration.t.sol b/test/VaultIntegration.t.sol index accda20..640a2fd 100644 --- a/test/VaultIntegration.t.sol +++ b/test/VaultIntegration.t.sol @@ -253,19 +253,79 @@ contract VaultIntegrationTest is Test { assertApproxEqRel(feeValue, 500e18, 0.02e18); } - function test_performanceFee_onTermGainsAboveProtected() public { + function test_performanceFee_onGainsAboveHighWater() public { vm.prank(owner); - vault.setFees(0, 1000, feeTo); // 10% of gains above the protected amount + vault.setFees(0, 1000, feeTo); // 10% above the high-water mark _enter(100_000e6); - prices.setEth(3000e18); // nav ~113.5k, protected 90k, gain ~23.5k + prices.setEth(3000e18); // nav ~113.5k; TRUE profit vs 100k start ~13.5k vm.warp(block.timestamp + 365 days + 1); vm.prank(keeper); vault.settleTerm(); - // fee shares self-dilute: value = feeWad x (nav - feeWad) / nav + // fee is 10% of REAL profit (nav - startNav), not of nav-above-floor; + // slightly less than 1353 after self-dilution of the minted fee shares uint256 feeValue = vault.balanceOf(feeTo) * vault.navPerShare() / 1e18; - assertApproxEqRel(feeValue, 2_304e18, 0.005e18); // 10% x 23.53k, diluted + assertApproxEqRel(feeValue, 1_337e18, 0.01e18); // 10% x ~13.5k, diluted + } + + // ---------- H1/H2 regression: fee basis + anti-churn ---------- + + function test_h1_noPerfFeeOnFlatTerm() public { + vm.prank(owner); + vault.setFees(0, 1000, feeTo); + _enter(100_000e6); + // no price move: navPerShare stays at the high-water mark + vm.warp(block.timestamp + 365 days + 1); + vm.prank(keeper); + vault.settleTerm(); + assertEq(vault.balanceOf(feeTo), 0); // flat term pays nothing + } + + function test_h1_noPerfFeeOnLosingTerm() public { + vm.prank(owner); + vault.setFees(0, 1000, feeTo); + _enter(100_000e6); + // ETH drifts down: NAV ends below start but above the floor + prices.setEth(1800e18); + vm.warp(block.timestamp + 365 days + 1); + vm.prank(keeper); + vault.settleTerm(); + assertEq(vault.balanceOf(feeTo), 0); // a losing vault pays no perf fee + } + + function test_h1_highWaterHoldsAcrossTerms() public { + vm.prank(owner); + vault.setFees(0, 1000, feeTo); + _enter(100_000e6); + + // term 1: rally, fee charged, HWM ratchets up + prices.setEth(3000e18); + vm.warp(block.timestamp + 365 days + 1); + vm.prank(keeper); + vault.settleTerm(); + uint256 feeAfterTerm1 = vault.balanceOf(feeTo); + assertGt(feeAfterTerm1, 0); + + // term 2 at the elevated NAV, then a flat close: no new gain above HWM + vm.prank(keeper); + vault.startTerm(365 days); + vault.rebalance(); + vm.warp(block.timestamp + 365 days + 1); + vm.prank(keeper); + vault.settleTerm(); + assertEq(vault.balanceOf(feeTo), feeAfterTerm1); // no double-charge + } + + function test_h2_minTermDuration_blocksChurn() public { + _enter(100_000e6); + vm.warp(block.timestamp + 365 days + 1); + vm.prank(keeper); + vault.settleTerm(); + // a one-second churn term is rejected + vm.prank(keeper); + vm.expectRevert(CPPIController.TermTooShort.selector); + vault.startTerm(1); } function test_feeCaps_enforced() public {