fix: address review feedback on PositionTracker and StrategyExecutor
- Fix fee attribution: distribute fees only to positions whose tick range contains the active tick at close time (in-range weight), not by raw liquidity. FLOOR is priced far below current tick and rarely earns fees; the old approach would over-credit it and corrupt capital-efficiency and net-P&L numbers. Fallback to raw-liquidity weighting with a WARN log when no position is in range. - Warn on first-close skip: when _closePosition finds no open record (first recenter, before any tracking), log [TRACKER][WARN] instead of silently returning so the gap is visible in reports. - Add tick range assertion: require() that the incoming close snapshot tick range matches the stored open record — a mismatch would mean IL is computed across different ranges (apples vs oranges). - Fix finalBlock accuracy: logSummary now calls tracker.logFinalSummary(tracker.lastNotifiedBlock()) instead of lastRecenterBlock, so the summary reflects the actual last replay block rather than potentially hundreds of blocks early. - Initialize lastRecenterBlock = block.number in StrategyExecutor constructor to defer the first recenter attempt by recenterInterval blocks and document the invariant. - Extract shared FormatLib: _str(uint256) and _istr(int256) were copy-pasted in both PositionTracker and StrategyExecutor. Extracted to FormatLib.sol internal library; both contracts now use `using FormatLib`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
openhands
parent
cfcf750084
commit
cf8e7ee6ee
3 changed files with 159 additions and 106 deletions
|
|
@ -1,9 +1,9 @@
|
|||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
pragma solidity ^0.8.19;
|
||||
|
||||
import { FormatLib } from "./FormatLib.sol";
|
||||
import { LiquidityAmounts } from "@aperture/uni-v3-lib/LiquidityAmounts.sol";
|
||||
import { TickMath } from "@aperture/uni-v3-lib/TickMath.sol";
|
||||
|
||||
import { Math } from "@openzeppelin/utils/math/Math.sol";
|
||||
import { IUniswapV3Pool } from "@uniswap-v3-core/interfaces/IUniswapV3Pool.sol";
|
||||
import { console2 } from "forge-std/console2.sol";
|
||||
|
|
@ -17,7 +17,10 @@ import { console2 } from "forge-std/console2.sol";
|
|||
* - Tick range (tickLower, tickUpper) and liquidity
|
||||
* - Entry/exit block and timestamp
|
||||
* - Token amounts at entry vs exit (via Uniswap V3 LiquidityAmounts math)
|
||||
* - Fees earned (proportional to each position's share of total liquidity)
|
||||
* - Fees earned — attributed only to positions whose tick range contained the
|
||||
* active tick at the time of close (the only positions that could have been
|
||||
* accruing fees). Falls back to raw-liquidity weighting if no position is
|
||||
* in range at close time (rare edge case).
|
||||
* - Impermanent loss vs holding the initial token amounts
|
||||
* - Net P&L = fees value + IL (IL is negative when LP underperforms HODL)
|
||||
*
|
||||
|
|
@ -26,15 +29,20 @@ import { console2 } from "forge-std/console2.sol";
|
|||
* - Cumulative IL and net P&L (token0 units)
|
||||
* - Rebalance count
|
||||
* - Time in range: % of notified blocks where Anchor tick range contains current tick
|
||||
* - Capital efficiency numerator/denominator for offline calculation
|
||||
* - Capital efficiency accumulators (feesToken0 / liqBlocks) for offline calculation
|
||||
*
|
||||
* All output lines carry a [TRACKER][TYPE] prefix for downstream parseability.
|
||||
* Cumulative P&L is logged every CUMULATIVE_LOG_INTERVAL blocks.
|
||||
*
|
||||
* Not a Script — no vm access. Uses console2 for output only.
|
||||
*
|
||||
* WARNING — backtesting use only: `recordRecenter` has no access control. Do not
|
||||
* reuse this contract in production contexts where untrusted callers exist.
|
||||
*/
|
||||
contract PositionTracker {
|
||||
using Math for uint256;
|
||||
using FormatLib for uint256;
|
||||
using FormatLib for int256;
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Types
|
||||
|
|
@ -167,6 +175,14 @@ contract PositionTracker {
|
|||
|
||||
/**
|
||||
* @notice Record a successful recenter: close old positions, open new ones.
|
||||
*
|
||||
* Fee attribution: fees are distributed only to positions whose tick range
|
||||
* contained the active tick at the moment of close — the only positions that
|
||||
* could have been accruing fees in Uniswap V3. When no position is in range
|
||||
* at close time (rare), attribution falls back to raw liquidity weighting and
|
||||
* a warning is logged.
|
||||
*
|
||||
* @dev No access control — this contract is backtesting-only tooling.
|
||||
* @param oldPos Pre-recenter snapshot (positions being burned).
|
||||
* @param newPos Post-recenter snapshot (positions being minted).
|
||||
* @param feesWeth Total WETH fees collected across all positions this recenter.
|
||||
|
|
@ -184,7 +200,7 @@ contract PositionTracker {
|
|||
)
|
||||
external
|
||||
{
|
||||
(uint160 sqrtPriceX96,,,,,,) = pool.slot0();
|
||||
(uint160 sqrtPriceX96, int24 currentTick,,,,,) = pool.slot0();
|
||||
|
||||
// Map WETH/KRK fees to pool token0/token1 based on pool ordering.
|
||||
uint256 fees0 = token0isWeth ? feesWeth : feesKrk;
|
||||
|
|
@ -195,12 +211,26 @@ contract PositionTracker {
|
|||
totalFeesToken0 += _valueInToken0(fees0, fees1, sqrtPriceX96);
|
||||
rebalanceCount++;
|
||||
|
||||
uint256 totalOldLiq = uint256(oldPos.floorLiq) + uint256(oldPos.anchorLiq) + uint256(oldPos.discLiq);
|
||||
// Uniswap V3 only accrues fees to a position when the active tick is
|
||||
// inside its range. Weight fee attribution by in-range liquidity only.
|
||||
uint256 fFloorWeight = _inRangeLiq(oldPos.floorLiq, oldPos.floorLo, oldPos.floorHi, currentTick);
|
||||
uint256 fAnchorWeight = _inRangeLiq(oldPos.anchorLiq, oldPos.anchorLo, oldPos.anchorHi, currentTick);
|
||||
uint256 fDiscWeight = _inRangeLiq(oldPos.discLiq, oldPos.discLo, oldPos.discHi, currentTick);
|
||||
uint256 totalFeeWeight = fFloorWeight + fAnchorWeight + fDiscWeight;
|
||||
|
||||
// Close old positions (skip stages where old liquidity is zero or no open record exists).
|
||||
_closePosition(0, oldPos.floorLiq, oldPos.floorLo, oldPos.floorHi, fees0, fees1, totalOldLiq, sqrtPriceX96, blockNum);
|
||||
_closePosition(1, oldPos.anchorLiq, oldPos.anchorLo, oldPos.anchorHi, fees0, fees1, totalOldLiq, sqrtPriceX96, blockNum);
|
||||
_closePosition(2, oldPos.discLiq, oldPos.discLo, oldPos.discHi, fees0, fees1, totalOldLiq, sqrtPriceX96, blockNum);
|
||||
// Fallback to raw-liquidity weights when no position covers the current tick.
|
||||
if (totalFeeWeight == 0) {
|
||||
console2.log("[TRACKER][WARN] no position in range at close: fee attribution falling back to raw liquidity");
|
||||
fFloorWeight = oldPos.floorLiq;
|
||||
fAnchorWeight = oldPos.anchorLiq;
|
||||
fDiscWeight = oldPos.discLiq;
|
||||
totalFeeWeight = uint256(oldPos.floorLiq) + uint256(oldPos.anchorLiq) + uint256(oldPos.discLiq);
|
||||
}
|
||||
|
||||
// Close old positions, passing per-position fee weights.
|
||||
_closePosition(0, oldPos.floorLiq, oldPos.floorLo, oldPos.floorHi, fees0, fees1, fFloorWeight, totalFeeWeight, sqrtPriceX96, blockNum);
|
||||
_closePosition(1, oldPos.anchorLiq, oldPos.anchorLo, oldPos.anchorHi, fees0, fees1, fAnchorWeight, totalFeeWeight, sqrtPriceX96, blockNum);
|
||||
_closePosition(2, oldPos.discLiq, oldPos.discLo, oldPos.discHi, fees0, fees1, fDiscWeight, totalFeeWeight, sqrtPriceX96, blockNum);
|
||||
|
||||
// Open new positions.
|
||||
_openPosition(0, newPos.floorLiq, newPos.floorLo, newPos.floorHi, sqrtPriceX96, blockNum, timestamp);
|
||||
|
|
@ -229,18 +259,18 @@ contract PositionTracker {
|
|||
uint256 timeInRangeBps = blocksChecked > 0 ? (blocksAnchorInRange * 10_000) / blocksChecked : 0;
|
||||
|
||||
console2.log("[TRACKER][SUMMARY] === Final P&L Summary ===");
|
||||
console2.log(string.concat("[TRACKER][SUMMARY] finalBlock=", _str(blockNum)));
|
||||
console2.log(string.concat("[TRACKER][SUMMARY] rebalances=", _str(rebalanceCount)));
|
||||
console2.log(string.concat("[TRACKER][SUMMARY] finalBlock=", blockNum.str()));
|
||||
console2.log(string.concat("[TRACKER][SUMMARY] rebalances=", rebalanceCount.str()));
|
||||
console2.log(
|
||||
string.concat("[TRACKER][SUMMARY] totalFees0=", _str(totalFees0), " totalFees1=", _str(totalFees1), " totalFeesToken0=", _str(totalFeesToken0))
|
||||
string.concat("[TRACKER][SUMMARY] totalFees0=", totalFees0.str(), " totalFees1=", totalFees1.str(), " totalFeesToken0=", totalFeesToken0.str())
|
||||
);
|
||||
console2.log(string.concat("[TRACKER][SUMMARY] totalIL=", _istr(finalIL), " netPnL=", _istr(finalNetPnL), " (token0 units)"));
|
||||
console2.log(string.concat("[TRACKER][SUMMARY] totalIL=", finalIL.istr(), " netPnL=", finalNetPnL.istr(), " (token0 units)"));
|
||||
console2.log(
|
||||
string.concat(
|
||||
"[TRACKER][SUMMARY] timeInRange=", _str(timeInRangeBps), " bps blocksAnchorInRange=", _str(blocksAnchorInRange), "/", _str(blocksChecked)
|
||||
"[TRACKER][SUMMARY] timeInRange=", timeInRangeBps.str(), " bps blocksAnchorInRange=", blocksAnchorInRange.str(), "/", blocksChecked.str()
|
||||
)
|
||||
);
|
||||
console2.log(string.concat("[TRACKER][SUMMARY] capitalEff: feesToken0=", _str(totalFeesToken0), " liqBlocks=", _str(totalLiquidityBlocks)));
|
||||
console2.log(string.concat("[TRACKER][SUMMARY] capitalEff: feesToken0=", totalFeesToken0.str(), " liqBlocks=", totalLiquidityBlocks.str()));
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
|
|
@ -275,19 +305,19 @@ contract PositionTracker {
|
|||
"[TRACKER][OPEN] stage=",
|
||||
_stageName(stageIdx),
|
||||
" block=",
|
||||
_str(blockNum),
|
||||
blockNum.str(),
|
||||
" ts=",
|
||||
_str(timestamp),
|
||||
timestamp.str(),
|
||||
" tick=[",
|
||||
_istr(tickLower),
|
||||
int256(tickLower).istr(),
|
||||
",",
|
||||
_istr(tickUpper),
|
||||
int256(tickUpper).istr(),
|
||||
"] liq=",
|
||||
_str(uint256(liquidity)),
|
||||
uint256(liquidity).str(),
|
||||
" amt0=",
|
||||
_str(amt0),
|
||||
amt0.str(),
|
||||
" amt1=",
|
||||
_str(amt1)
|
||||
amt1.str()
|
||||
)
|
||||
);
|
||||
}
|
||||
|
|
@ -299,22 +329,35 @@ contract PositionTracker {
|
|||
int24 tickUpper,
|
||||
uint256 fees0Total,
|
||||
uint256 fees1Total,
|
||||
uint256 totalOldLiq,
|
||||
uint256 posWeight, // fee weight for this position (0 = out-of-range)
|
||||
uint256 totalWeight, // sum of fee weights across all positions
|
||||
uint160 sqrtPriceX96,
|
||||
uint256 blockNum
|
||||
)
|
||||
internal
|
||||
{
|
||||
if (liquidity == 0) return;
|
||||
|
||||
OpenPosition storage pos = openPositions[stageIdx];
|
||||
if (!pos.active) return; // first ever close: no open record to match
|
||||
if (!pos.active) {
|
||||
// First recenter: no prior open record exists (LM deployed with positions
|
||||
// already placed before tracking began). Log a warning so the gap is visible.
|
||||
console2.log(
|
||||
string.concat("[TRACKER][WARN] stage=", _stageName(stageIdx), " close skipped at block=", blockNum.str(), " (no open record: first recenter)")
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Guard: the incoming snapshot must match the stored open record.
|
||||
// A mismatch would mean IL is computed across mismatched tick ranges.
|
||||
require(tickLower == pos.tickLower && tickUpper == pos.tickUpper, "PositionTracker: tick range mismatch");
|
||||
|
||||
(uint256 exitAmt0, uint256 exitAmt1) = _positionAmounts(liquidity, tickLower, tickUpper, sqrtPriceX96);
|
||||
|
||||
// Attribute fees proportional to this position's share of total liquidity.
|
||||
// Attribute fees proportional to this position's in-range weight.
|
||||
uint256 posLiq = uint256(liquidity);
|
||||
uint256 myFees0 = totalOldLiq > 0 ? fees0Total.mulDiv(posLiq, totalOldLiq) : 0;
|
||||
uint256 myFees1 = totalOldLiq > 0 ? fees1Total.mulDiv(posLiq, totalOldLiq) : 0;
|
||||
uint256 myFees0 = totalWeight > 0 ? fees0Total.mulDiv(posWeight, totalWeight) : 0;
|
||||
uint256 myFees1 = totalWeight > 0 ? fees1Total.mulDiv(posWeight, totalWeight) : 0;
|
||||
|
||||
// IL = LP exit value (ex-fees) − HODL value at exit price (both in token0 units).
|
||||
int256 il = _computeIL(pos.entryAmount0, pos.entryAmount1, exitAmt0, exitAmt1, sqrtPriceX96);
|
||||
|
|
@ -332,34 +375,34 @@ contract PositionTracker {
|
|||
"[TRACKER][CLOSE] stage=",
|
||||
_stageName(stageIdx),
|
||||
" block=",
|
||||
_str(blockNum),
|
||||
blockNum.str(),
|
||||
" entryBlock=",
|
||||
_str(pos.entryBlock),
|
||||
pos.entryBlock.str(),
|
||||
" tick=[",
|
||||
_istr(tickLower),
|
||||
int256(tickLower).istr(),
|
||||
",",
|
||||
_istr(tickUpper),
|
||||
int256(tickUpper).istr(),
|
||||
"] liq=",
|
||||
_str(posLiq)
|
||||
posLiq.str()
|
||||
)
|
||||
);
|
||||
console2.log(
|
||||
string.concat(
|
||||
"[TRACKER][CLOSE] entryAmt0=",
|
||||
_str(pos.entryAmount0),
|
||||
pos.entryAmount0.str(),
|
||||
" entryAmt1=",
|
||||
_str(pos.entryAmount1),
|
||||
pos.entryAmount1.str(),
|
||||
" exitAmt0=",
|
||||
_str(exitAmt0),
|
||||
exitAmt0.str(),
|
||||
" exitAmt1=",
|
||||
_str(exitAmt1),
|
||||
exitAmt1.str(),
|
||||
" fees0=",
|
||||
_str(myFees0),
|
||||
myFees0.str(),
|
||||
" fees1=",
|
||||
_str(myFees1)
|
||||
myFees1.str()
|
||||
)
|
||||
);
|
||||
console2.log(string.concat("[TRACKER][CLOSE] IL=", _istr(il), " netPnL=", _istr(netPnL), " (token0 units)"));
|
||||
console2.log(string.concat("[TRACKER][CLOSE] IL=", il.istr(), " netPnL=", netPnL.istr(), " (token0 units)"));
|
||||
|
||||
delete openPositions[stageIdx];
|
||||
}
|
||||
|
|
@ -369,19 +412,19 @@ contract PositionTracker {
|
|||
console2.log(
|
||||
string.concat(
|
||||
"[TRACKER][CUMULATIVE] block=",
|
||||
_str(blockNum),
|
||||
blockNum.str(),
|
||||
" rebalances=",
|
||||
_str(rebalanceCount),
|
||||
rebalanceCount.str(),
|
||||
" totalFees0=",
|
||||
_str(totalFees0),
|
||||
totalFees0.str(),
|
||||
" totalFees1=",
|
||||
_str(totalFees1),
|
||||
totalFees1.str(),
|
||||
" IL=",
|
||||
_istr(totalILToken0),
|
||||
totalILToken0.istr(),
|
||||
" netPnL=",
|
||||
_istr(totalNetPnLToken0),
|
||||
totalNetPnLToken0.istr(),
|
||||
" timeInRange=",
|
||||
_str(timeInRangeBps),
|
||||
timeInRangeBps.str(),
|
||||
" bps"
|
||||
)
|
||||
);
|
||||
|
|
@ -391,6 +434,15 @@ contract PositionTracker {
|
|||
// Internal: Uniswap V3 math
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* @notice Return `liquidity` if currentTick is inside [tickLo, tickHi), else 0.
|
||||
* Used to attribute fees only to positions that were potentially in range.
|
||||
*/
|
||||
function _inRangeLiq(uint128 liquidity, int24 tickLo, int24 tickHi, int24 currentTick) internal pure returns (uint256) {
|
||||
if (liquidity == 0) return 0;
|
||||
return (currentTick >= tickLo && currentTick < tickHi) ? uint256(liquidity) : 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* @notice Compute (amount0, amount1) for a liquidity position at the given sqrt price.
|
||||
*/
|
||||
|
|
@ -433,7 +485,7 @@ contract PositionTracker {
|
|||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Formatting helpers (no vm dependency)
|
||||
// Formatting helper
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
function _stageName(uint8 idx) internal pure returns (string memory) {
|
||||
|
|
@ -441,25 +493,4 @@ contract PositionTracker {
|
|||
if (idx == 1) return "ANCHOR";
|
||||
return "DISC";
|
||||
}
|
||||
|
||||
function _str(uint256 v) internal pure returns (string memory) {
|
||||
if (v == 0) return "0";
|
||||
uint256 tmp = v;
|
||||
uint256 len;
|
||||
while (tmp != 0) {
|
||||
len++;
|
||||
tmp /= 10;
|
||||
}
|
||||
bytes memory buf = new bytes(len);
|
||||
while (v != 0) {
|
||||
buf[--len] = bytes1(uint8(48 + v % 10));
|
||||
v /= 10;
|
||||
}
|
||||
return string(buf);
|
||||
}
|
||||
|
||||
function _istr(int256 v) internal pure returns (string memory) {
|
||||
if (v >= 0) return _str(uint256(v));
|
||||
return string.concat("-", _str(uint256(-v)));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue