johba/harb
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: _scrapePositions: unguarded safeTransfer to address(0) when feeDestination unset (#624)

Browse source 
Add address(0) guard to fee transfer condition in _scrapePositions so
that when feeDestination is uninitialized, fees accrue as deployable
liquidity instead of reverting on safeTransfer to the zero address.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
openhands 2026-03-20 01:05:05 +00:00
parent 84fae1a410
commit 170cc839e6
3 changed files with 98 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

56
onchain/test/LiquidityManager.t.sol
View file

@ -1164,6 +1164,62 @@ contract LiquidityManagerTest is UniSwapHelper {
assertEq(weth.balanceOf(makeAddr("stakeFeeDestination")), 0, "No WETH to stake fee address");
}
/**
* @notice When feeDestination is unset (address(0)), recenter must not revert.
* Fees accrue as deployable liquidity instead of attempting safeTransfer to address(0).
*
* Regression test for issue #624: without the fix, _scrapePositions checks
* `feeDestination != address(this)` which is true for address(0), causing
* safeTransfer to address(0) which reverts in OpenZeppelin ERC20.
*/
function testRecenterWithUnsetFeeDestination_FeesAccrueAsLiquidity() public {
disableAutoSetup();
// Deploy a fresh environment where setFeeDestination is never called
TestEnvironment zeroFeeEnv = new TestEnvironment(makeAddr("unused"));
(
IUniswapV3Factory _factory,
IUniswapV3Pool _pool,
IWETH9 _weth,
Kraiken _harberg,
,
LiquidityManager _lm,
Optimizer _optimizer,
bool _token0isWeth
) = zeroFeeEnv.setupEnvironmentWithUnsetFeeDestination(DEFAULT_TOKEN0_IS_WETH);
// Wire state variables used by buy/sell helpers
factory = _factory;
pool = _pool;
weth = _weth;
harberg = _harberg;
lm = _lm;
optimizer = _optimizer;
token0isWeth = _token0isWeth;
assertEq(lm.feeDestination(), address(0), "precondition: feeDestination must be unset");
// Fund the trader account
vm.deal(account, 100 ether);
vm.prank(account);
weth.deposit{ value: 100 ether }();
// First recenter: creates positions (no scrape needed, no fees yet)
vm.prank(RECENTER_CALLER);
lm.recenter();
// Generate fees via a buy
buyRaw(10 ether);
// Warp past cooldown + TWAP settlement
vm.warp(block.timestamp + 301);
// Second recenter: _scrapePositions runs with non-zero fees.
// Without the fix this reverts on safeTransfer to address(0).
vm.prank(RECENTER_CALLER);
lm.recenter(); // must not revert
}
/**
* @notice Deploy a LiquidityManagerHarness and call the exposed abstract functions
* to cover _getKraikenToken() and _getWethToken() (lines 270-271, 275-276)

40
onchain/test/helpers/TestBase.sol
View file

@ -284,6 +284,46 @@ contract TestEnvironment is TestConstants {
return (factory, pool, weth, harberg, stake, lm, optimizer, token0isWeth);
}
/**
* @notice Deploy all contracts WITHOUT calling setFeeDestination — feeDestination stays address(0).
* @dev Regression helper for issue #624: verifies that _scrapePositions gracefully handles
* an uninitialized feeDestination instead of attempting safeTransfer to address(0).
* @param token0shouldBeWeth Whether WETH should be token0
*/
function setupEnvironmentWithUnsetFeeDestination(bool token0shouldBeWeth)
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));
// Intentionally do NOT call lm.setFeeDestination() — feeDestination stays address(0)
harberg.setStakingPool(address(stake));
harberg.setLiquidityManager(address(lm));
vm.deal(address(lm), INITIAL_LM_ETH_BALANCE);
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