fix: No events on fee destination state changes (#958)

Add FeeDestinationSet and FeeDestinationLocked events to LiquidityManager,
emitted on every setFeeDestination() call and lock engagement respectively.
Update tests to assert both events are emitted in all code paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
openhands 2026-03-18 21:25:12 +00:00
parent 240d3ae1ac
commit e3c699b7eb
2 changed files with 33 additions and 3 deletions

View file

@ -65,6 +65,12 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle {
/// @notice Emitted on each successful recenter for monitoring and indexing /// @notice Emitted on each successful recenter for monitoring and indexing
event Recentered(int24 indexed currentTick, bool indexed isUp); event Recentered(int24 indexed currentTick, bool indexed isUp);
/// @notice Emitted whenever feeDestination is updated
event FeeDestinationSet(address indexed newDest);
/// @notice Emitted when the fee destination lock is permanently engaged
event FeeDestinationLocked(address indexed dest);
/// @notice Custom errors /// @notice Custom errors
error ZeroAddressInSetter(); error ZeroAddressInSetter();
@ -139,12 +145,15 @@ contract LiquidityManager is ThreePositionStrategy, PriceOracle {
// to storage. A subsequent SELFDESTRUCT clears the bytecode but cannot undo this write. // to storage. A subsequent SELFDESTRUCT clears the bytecode but cannot undo this write.
if (!feeDestinationLocked && feeDestination != address(0) && feeDestination.code.length > 0) { if (!feeDestinationLocked && feeDestination != address(0) && feeDestination.code.length > 0) {
feeDestinationLocked = true; feeDestinationLocked = true;
emit FeeDestinationLocked(feeDestination);
return; return;
} }
require(!feeDestinationLocked, "fee destination locked"); require(!feeDestinationLocked, "fee destination locked");
feeDestination = feeDestination_; feeDestination = feeDestination_;
emit FeeDestinationSet(feeDestination_);
if (feeDestination_.code.length > 0) { if (feeDestination_.code.length > 0) {
feeDestinationLocked = true; feeDestinationLocked = true;
emit FeeDestinationLocked(feeDestination_);
} }
} }

View file

@ -979,9 +979,15 @@ contract LiquidityManagerTest is UniSwapHelper {
*/ */
function testSetFeeDestinationEOA_MultipleAllowed() public { function testSetFeeDestinationEOA_MultipleAllowed() public {
LiquidityManager freshLm = new LiquidityManager(address(factory), address(weth), address(harberg), address(optimizer)); LiquidityManager freshLm = new LiquidityManager(address(factory), address(weth), address(harberg), address(optimizer));
freshLm.setFeeDestination(makeAddr("firstFee")); address firstFee = makeAddr("firstFee");
address secondFee = makeAddr("secondFee");
vm.expectEmit(true, false, false, false, address(freshLm));
emit LiquidityManager.FeeDestinationSet(firstFee);
freshLm.setFeeDestination(firstFee);
assertFalse(freshLm.feeDestinationLocked(), "should not be locked after EOA set"); assertFalse(freshLm.feeDestinationLocked(), "should not be locked after EOA set");
freshLm.setFeeDestination(makeAddr("secondFee")); vm.expectEmit(true, false, false, false, address(freshLm));
emit LiquidityManager.FeeDestinationSet(secondFee);
freshLm.setFeeDestination(secondFee);
assertFalse(freshLm.feeDestinationLocked(), "should still not be locked after second EOA set"); assertFalse(freshLm.feeDestinationLocked(), "should still not be locked after second EOA set");
} }
@ -991,6 +997,10 @@ contract LiquidityManagerTest is UniSwapHelper {
function testSetFeeDestinationContract_Locks() public { function testSetFeeDestinationContract_Locks() public {
LiquidityManager freshLm = new LiquidityManager(address(factory), address(weth), address(harberg), address(optimizer)); LiquidityManager freshLm = new LiquidityManager(address(factory), address(weth), address(harberg), address(optimizer));
// address(harberg) is a deployed contract // address(harberg) is a deployed contract
vm.expectEmit(true, false, false, false, address(freshLm));
emit LiquidityManager.FeeDestinationSet(address(harberg));
vm.expectEmit(true, false, false, false, address(freshLm));
emit LiquidityManager.FeeDestinationLocked(address(harberg));
freshLm.setFeeDestination(address(harberg)); freshLm.setFeeDestination(address(harberg));
assertTrue(freshLm.feeDestinationLocked(), "should be locked after contract set"); assertTrue(freshLm.feeDestinationLocked(), "should be locked after contract set");
assertEq(freshLm.feeDestination(), address(harberg)); assertEq(freshLm.feeDestination(), address(harberg));
@ -1001,10 +1011,17 @@ contract LiquidityManagerTest is UniSwapHelper {
*/ */
function testSetFeeDestinationEOAToContract_Locks() public { function testSetFeeDestinationEOAToContract_Locks() public {
LiquidityManager freshLm = new LiquidityManager(address(factory), address(weth), address(harberg), address(optimizer)); LiquidityManager freshLm = new LiquidityManager(address(factory), address(weth), address(harberg), address(optimizer));
address treasuryEOA = makeAddr("treasuryEOA");
// Step 1: set to an EOA during setup allowed, not locked // Step 1: set to an EOA during setup allowed, not locked
freshLm.setFeeDestination(makeAddr("treasuryEOA")); vm.expectEmit(true, false, false, false, address(freshLm));
emit LiquidityManager.FeeDestinationSet(treasuryEOA);
freshLm.setFeeDestination(treasuryEOA);
assertFalse(freshLm.feeDestinationLocked(), "not locked after EOA set"); assertFalse(freshLm.feeDestinationLocked(), "not locked after EOA set");
// Step 2: upgrade to treasury contract once it is deployed locks permanently // Step 2: upgrade to treasury contract once it is deployed locks permanently
vm.expectEmit(true, false, false, false, address(freshLm));
emit LiquidityManager.FeeDestinationSet(address(harberg));
vm.expectEmit(true, false, false, false, address(freshLm));
emit LiquidityManager.FeeDestinationLocked(address(harberg));
freshLm.setFeeDestination(address(harberg)); freshLm.setFeeDestination(address(harberg));
assertTrue(freshLm.feeDestinationLocked(), "locked after contract set"); assertTrue(freshLm.feeDestinationLocked(), "locked after contract set");
assertEq(freshLm.feeDestination(), address(harberg)); assertEq(freshLm.feeDestination(), address(harberg));
@ -1033,6 +1050,8 @@ contract LiquidityManagerTest is UniSwapHelper {
address eoaAddr = makeAddr("precomputedEOA"); address eoaAddr = makeAddr("precomputedEOA");
// Step 1: set to EOA allowed, lock stays false // Step 1: set to EOA allowed, lock stays false
vm.expectEmit(true, false, false, false, address(freshLm));
emit LiquidityManager.FeeDestinationSet(eoaAddr);
freshLm.setFeeDestination(eoaAddr); freshLm.setFeeDestination(eoaAddr);
assertFalse(freshLm.feeDestinationLocked(), "not locked after EOA set"); assertFalse(freshLm.feeDestinationLocked(), "not locked after EOA set");
@ -1043,6 +1062,8 @@ contract LiquidityManagerTest is UniSwapHelper {
// Step 3: calling setFeeDestination detects bytecode at the current destination and // Step 3: calling setFeeDestination detects bytecode at the current destination and
// commits feeDestinationLocked = true WITHOUT reverting, so the write survives // commits feeDestinationLocked = true WITHOUT reverting, so the write survives
// a later SELFDESTRUCT. feeDestination itself is not changed. // a later SELFDESTRUCT. feeDestination itself is not changed.
vm.expectEmit(true, false, false, false, address(freshLm));
emit LiquidityManager.FeeDestinationLocked(eoaAddr);
freshLm.setFeeDestination(makeAddr("attacker")); freshLm.setFeeDestination(makeAddr("attacker"));
assertTrue(freshLm.feeDestinationLocked(), "locked after bytecode detected at current feeDestination"); assertTrue(freshLm.feeDestinationLocked(), "locked after bytecode detected at current feeDestination");
assertEq(freshLm.feeDestination(), eoaAddr, "feeDestination must not change when defensive lock triggers"); assertEq(freshLm.feeDestination(), eoaAddr, "feeDestination must not change when defensive lock triggers");