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

fix: fix: Conditional lock on feeDestination — lock when set to contract (#580) (#580)

Browse source 
- Add `feeDestinationLocked` bool to LiquidityManager
- Replace one-shot setter with conditional trapdoor: EOAs may be set
  repeatedly, but setting a contract address locks permanently
- Remove `AddressAlreadySet` error (superseded by the new lock mechanic)
- Replace fragile SLOT7 storage hack in red-team.sh with a proper
  `setFeeDestination()` call using the deployer key
- Update tests: replace AddressAlreadySet test with three new tests
  covering EOA multi-set, contract lock, and locked revert

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
openhands 2026-03-12 16:13:44 +00:00
parent fff0ca60e7
commit 512640226b
3 changed files with 39 additions and 19 deletions
Download patch file Download diff file Expand all files Collapse all files

10
onchain/src/LiquidityManager.sol
View file

@ -47,6 +47,7 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle {
address private immutable deployer;
address public recenterAccess;
address public feeDestination;
bool public feeDestinationLocked;
/// @notice Last recenter tick — used to determine net trade direction between recenters
int24 public lastRecenterTick;
@ -63,7 +64,6 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle {
/// @notice Custom errors
error ZeroAddressInSetter();
error AddressAlreadySet();
/// @notice Access control modifier
modifier onlyFeeDestination() {
@ -114,13 +114,17 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle {
if (amount1Owed > 0) IERC20(poolKey.token1).safeTransfer(msg.sender, amount1Owed);
}
/// @notice Sets the fee destination address (can only be called once, deployer only)
/// @notice Sets the fee destination address (deployer only)
/// @dev Any EOA can be set repeatedly. Once a contract address is set, locks permanently.
/// @param feeDestination_ The address that will receive trading fees
function setFeeDestination(address feeDestination_) external {
require(msg.sender == deployer, "only deployer");
if (address(0) == feeDestination_) revert ZeroAddressInSetter();
if (feeDestination != address(0)) revert AddressAlreadySet();
require(!feeDestinationLocked, "fee destination locked");
feeDestination = feeDestination_;
if (feeDestination_.code.length > 0) {
feeDestinationLocked = true;
}
}
/// @notice Sets recenter access for testing/emergency purposes

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

@ -1017,14 +1017,35 @@ contract LiquidityManagerTest is UniSwapHelper {
}
/**
* @notice AddressAlreadySet revert when fee destination is changed after initial set
* @notice EOA destinations can be set repeatedly — no lock until a contract is set
*/
function testSetFeeDestinationAlreadySet() public {
// Deploy a fresh LM with this test contract as deployer
function testSetFeeDestinationEOA_MultipleAllowed() public {
LiquidityManager freshLm = new LiquidityManager(address(factory), address(weth), address(harberg), address(optimizer));
freshLm.setFeeDestination(makeAddr("firstFee"));
vm.expectRevert(LiquidityManager.AddressAlreadySet.selector);
assertFalse(freshLm.feeDestinationLocked(), "should not be locked after EOA set");
freshLm.setFeeDestination(makeAddr("secondFee"));
assertFalse(freshLm.feeDestinationLocked(), "should still not be locked after second EOA set");
}
/**
* @notice Setting fee destination to a contract locks it permanently
*/
function testSetFeeDestinationContract_Locks() public {
LiquidityManager freshLm = new LiquidityManager(address(factory), address(weth), address(harberg), address(optimizer));
// address(harberg) is a deployed contract
freshLm.setFeeDestination(address(harberg));
assertTrue(freshLm.feeDestinationLocked(), "should be locked after contract set");
assertEq(freshLm.feeDestination(), address(harberg));
}
/**
* @notice Once locked, setFeeDestination reverts
*/
function testSetFeeDestinationLocked_Reverts() public {
LiquidityManager freshLm = new LiquidityManager(address(factory), address(weth), address(harberg), address(optimizer));
freshLm.setFeeDestination(address(harberg));
vm.expectRevert("fee destination locked");
freshLm.setFeeDestination(makeAddr("anyAddr"));
}
/**