fix: LM: accrue fees as liquidity when feeDestination is self (#533)
When feeDestination == address(this), _scrapePositions() now skips the fee safeTransfer calls so collected WETH/KRK stays in the LM balance and is redeployed as liquidity on the next _setPositions() call. Also fixes _getOutstandingSupply(): kraiken.outstandingSupply() already subtracts balanceOf(liquidityManager), so when feeDestination IS the LM the old code double-subtracted LM-held KRK, causing an arithmetic underflow once positions were scraped. The subtraction is now skipped for the self-referencing case. VWAP recording is refactored to a single unconditional block so it fires regardless of fee destination. New test testSelfFeeDestination_FeesAccrueAsLiquidity() demonstrates that a two-recenter cycle with self-feeDestination completes without underflow and without leaking WETH to any external address. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
openhands
parent
13d40222b6
commit
2c21462e1e
3 changed files with 135 additions and 15 deletions
|
|
@ -234,23 +234,29 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle {
|
|||
|
||||
// Transfer fees and record volume for VWAP
|
||||
// Only record VWAP when net ETH inflow (KRK sold out) — prevents sell-back
|
||||
// activity from diluting the price memory of original KRK distribution
|
||||
if (fee0 > 0) {
|
||||
if (token0isWeth) {
|
||||
IERC20(address(weth)).safeTransfer(feeDestination, fee0);
|
||||
if (recordVWAP) _recordVolumeAndPrice(currentPrice, fee0);
|
||||
} else {
|
||||
IERC20(address(kraiken)).safeTransfer(feeDestination, fee0);
|
||||
// activity from diluting the price memory of original KRK distribution.
|
||||
// Skip transfer when feeDestination is self — fees accrue as deployable liquidity.
|
||||
if (feeDestination != address(this)) {
|
||||
if (fee0 > 0) {
|
||||
if (token0isWeth) {
|
||||
IERC20(address(weth)).safeTransfer(feeDestination, fee0);
|
||||
} else {
|
||||
IERC20(address(kraiken)).safeTransfer(feeDestination, fee0);
|
||||
}
|
||||
}
|
||||
if (fee1 > 0) {
|
||||
if (token0isWeth) {
|
||||
IERC20(address(kraiken)).safeTransfer(feeDestination, fee1);
|
||||
} else {
|
||||
IERC20(address(weth)).safeTransfer(feeDestination, fee1);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (fee1 > 0) {
|
||||
if (token0isWeth) {
|
||||
IERC20(address(kraiken)).safeTransfer(feeDestination, fee1);
|
||||
} else {
|
||||
IERC20(address(weth)).safeTransfer(feeDestination, fee1);
|
||||
if (recordVWAP) _recordVolumeAndPrice(currentPrice, fee1);
|
||||
}
|
||||
// Always record VWAP regardless of fee destination
|
||||
if (recordVWAP) {
|
||||
uint256 ethFee = token0isWeth ? fee0 : fee1;
|
||||
if (ethFee > 0) _recordVolumeAndPrice(currentPrice, ethFee);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -297,7 +303,9 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle {
|
|||
/// since neither can be sold into the floor — only trader-held KRK matters for scarcity
|
||||
function _getOutstandingSupply() internal view override returns (uint256) {
|
||||
uint256 supply = kraiken.outstandingSupply();
|
||||
if (feeDestination != address(0)) {
|
||||
// Skip subtraction when feeDestination is self: outstandingSupply() already
|
||||
// excludes kraiken.balanceOf(address(this)), so subtracting again would double-count.
|
||||
if (feeDestination != address(0) && feeDestination != address(this)) {
|
||||
supply -= kraiken.balanceOf(feeDestination);
|
||||
}
|
||||
(, address stakingPoolAddr) = kraiken.peripheryContracts();
|
||||
|
|
|
|||
|
|
@ -1073,6 +1073,67 @@ contract LiquidityManagerTest is UniSwapHelper {
|
|||
assertTrue(lm.lastRecenterTick() != 0, "lastRecenterTick should have been updated after recenter");
|
||||
}
|
||||
|
||||
/**
|
||||
* @notice When feeDestination == address(lm), recenter must not transfer fees externally
|
||||
* and _getOutstandingSupply must not double-subtract LM-held KRK.
|
||||
*
|
||||
* Regression test for issue #533: without the fix, the second recenter (which scrapes
|
||||
* positions and temporarily puts principal KRK into the LM's balance) triggers an
|
||||
* arithmetic underflow in _getOutstandingSupply because kraiken.outstandingSupply()
|
||||
* already excludes balanceOf(lm) and the old code subtracted it a second time.
|
||||
*/
|
||||
function testSelfFeeDestination_FeesAccrueAsLiquidity() public {
|
||||
disableAutoSetup();
|
||||
|
||||
// Deploy a fresh environment where LM's own address is the feeDestination
|
||||
TestEnvironment selfFeeEnv = new TestEnvironment(makeAddr("unused"));
|
||||
(
|
||||
IUniswapV3Factory _factory,
|
||||
IUniswapV3Pool _pool,
|
||||
IWETH9 _weth,
|
||||
Kraiken _harberg,
|
||||
,
|
||||
LiquidityManager _lm,
|
||||
Optimizer _optimizer,
|
||||
bool _token0isWeth
|
||||
) = selfFeeEnv.setupEnvironmentWithSelfFeeDestination(DEFAULT_TOKEN0_IS_WETH, RECENTER_CALLER);
|
||||
|
||||
// Wire state variables used by buy/sell/recenter helpers
|
||||
factory = _factory;
|
||||
pool = _pool;
|
||||
weth = _weth;
|
||||
harberg = _harberg;
|
||||
lm = _lm;
|
||||
optimizer = _optimizer;
|
||||
token0isWeth = _token0isWeth;
|
||||
|
||||
assertEq(lm.feeDestination(), address(lm), "precondition: feeDestination must be self");
|
||||
|
||||
// Fund the trader account
|
||||
vm.deal(account, 100 ether);
|
||||
vm.prank(account);
|
||||
weth.deposit{ value: 100 ether }();
|
||||
|
||||
// First recenter: no existing positions, amplitude check skipped, positions created
|
||||
vm.prank(RECENTER_CALLER);
|
||||
lm.recenter();
|
||||
|
||||
// Move price up with a buy so the second recenter satisfies amplitude requirement
|
||||
buyRaw(10 ether);
|
||||
|
||||
// Second recenter: _scrapePositions() burns positions and collects principal KRK
|
||||
// into the LM's balance. _setPositions() then calls _getOutstandingSupply().
|
||||
// Without the fix: outstandingSupply() already excludes balanceOf(lm), and
|
||||
// the old code subtracts balanceOf(feeDestination)==balanceOf(lm) again → underflow.
|
||||
// With the fix: the second subtraction is skipped → no underflow.
|
||||
vm.prank(RECENTER_CALLER);
|
||||
lm.recenter(); // must not revert
|
||||
|
||||
// Verify no WETH leaked to any external address — the LM kept its own fees
|
||||
assertEq(weth.balanceOf(makeAddr("unused")), 0, "No WETH should have been sent to any external address");
|
||||
assertEq(weth.balanceOf(makeAddr("stakeFeeDestination")), 0, "No WETH to stake fee address");
|
||||
}
|
||||
|
||||
/**
|
||||
* @notice Deploy a LiquidityManagerHarness and call the exposed abstract functions
|
||||
* to cover _getKraikenToken() and _getWethToken() (lines 270-271, 275-276)
|
||||
|
|
|
|||
|
|
@ -255,6 +255,57 @@ contract TestEnvironment is TestConstants {
|
|||
return (factory, pool, weth, harberg, stake, lm, optimizer, token0isWeth);
|
||||
}
|
||||
|
||||
/**
|
||||
* @notice Deploy all contracts with feeDestination set to the LiquidityManager itself.
|
||||
* @dev Useful for testing the "fees accrue as liquidity" path introduced in issue #533.
|
||||
* When feeDestination == address(lm), _scrapePositions skips fee transfers and
|
||||
* _getOutstandingSupply skips the feeDestination KRK subtraction (already excluded
|
||||
* by outstandingSupply()).
|
||||
* @param token0shouldBeWeth Whether WETH should be token0
|
||||
* @param recenterCaller Address that will be granted recenter access
|
||||
*/
|
||||
function setupEnvironmentWithSelfFeeDestination(
|
||||
bool token0shouldBeWeth,
|
||||
address recenterCaller
|
||||
)
|
||||
external
|
||||
returns (
|
||||
IUniswapV3Factory _factory,
|
||||
IUniswapV3Pool _pool,
|
||||
IWETH9 _weth,
|
||||
Kraiken _harberg,
|
||||
Stake _stake,
|
||||
LiquidityManager _lm,
|
||||
Optimizer _optimizer,
|
||||
bool _token0isWeth
|
||||
)
|
||||
{
|
||||
// Deploy factory and tokens
|
||||
factory = UniswapHelpers.deployUniswapFactory();
|
||||
_deployTokensWithOrder(token0shouldBeWeth);
|
||||
_createAndInitializePool();
|
||||
|
||||
// Deploy protocol contracts; Stake gets a throwaway feeDestination address
|
||||
address stakeFee = makeAddr("stakeFeeDestination");
|
||||
stake = new Stake(address(harberg), stakeFee);
|
||||
optimizer = Optimizer(address(new MockOptimizer()));
|
||||
optimizer.initialize(address(harberg), address(stake));
|
||||
lm = new LiquidityManager(address(factory), address(weth), address(harberg), address(optimizer));
|
||||
|
||||
// Set feeDestination to the LM itself — this is the self-referencing mode under test
|
||||
lm.setFeeDestination(address(lm));
|
||||
|
||||
harberg.setStakingPool(address(stake));
|
||||
harberg.setLiquidityManager(address(lm));
|
||||
vm.deal(address(lm), INITIAL_LM_ETH_BALANCE);
|
||||
|
||||
// feeDestination IS address(lm), so prank as lm to grant recenter access
|
||||
vm.prank(address(lm));
|
||||
lm.setRecenterAccess(recenterCaller);
|
||||
|
||||
return (factory, pool, weth, harberg, stake, lm, optimizer, token0isWeth);
|
||||
}
|
||||
|
||||
/**
|
||||
* @notice Setup environment with existing factory and specific optimizer
|
||||
* @param existingFactory The existing Uniswap factory to use
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue