Skip to content

Commit 089baeb

Browse files
committed
added test that shows the bug, along with a fix
1 parent fdd68b0 commit 089baeb

File tree

2 files changed

+55
-0
lines changed

2 files changed

+55
-0
lines changed

packages/coins/src/libs/V4Liquidity.sol

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,17 @@ library V4Liquidity {
192192
for (uint256 i; i < positions.length; i++) {
193193
uint128 liquidity = getLiquidity(poolManager, address(this), poolKey, positions[i].tickLower, positions[i].tickUpper);
194194

195+
// Skip positions that have no liquidity to avoid CannotUpdateEmptyPosition error
196+
if (liquidity == 0) {
197+
burnedPositions[i] = BurnedPosition({
198+
tickLower: positions[i].tickLower,
199+
tickUpper: positions[i].tickUpper,
200+
amount0Received: 0,
201+
amount1Received: 0
202+
});
203+
continue;
204+
}
205+
195206
ModifyLiquidityParams memory params = ModifyLiquidityParams({
196207
tickLower: positions[i].tickLower,
197208
tickUpper: positions[i].tickUpper,

packages/coins/test/LiquidityMigration.t.sol

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ import {ICoin} from "../src/interfaces/ICoin.sol";
1717
import {Currency} from "@uniswap/v4-core/src/types/Currency.sol";
1818
import {CoinCommon} from "../src/libs/CoinCommon.sol";
1919
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
20+
import {MultiOwnable} from "../src/utils/MultiOwnable.sol";
2021
import {IHooksUpgradeGate} from "../src/interfaces/IHooksUpgradeGate.sol";
22+
import {BaseCoin} from "../src/BaseCoin.sol";
2123

2224
contract LiquidityMigrationReceiver is IUpgradeableDestinationV4Hook, IERC165 {
2325
function initializeFromMigration(
@@ -45,6 +47,8 @@ contract InvalidLiquidityMigrationReceiver is IERC165 {
4547
contract LiquidityMigrationTest is BaseTest {
4648
MockERC20 internal mockERC20A;
4749

50+
address constant coinVersionLookup = 0x777777751622c0d3258f214F9DF38E35BF45baF3;
51+
4852
function setUp() public override {
4953
super.setUpWithBlockNumber(30267794);
5054

@@ -388,4 +392,44 @@ contract LiquidityMigrationTest is BaseTest {
388392
// Should match isRegisteredUpgradePath
389393
assertEq(hookUpgradeGate.isAllowedHookUpgrade(baseImpl, upgradeImpl), hookUpgradeGate.isRegisteredUpgradePath(baseImpl, upgradeImpl));
390394
}
395+
396+
function test_migrateLiquidity_failsWithEmptyPositionBug() public {
397+
// Reproduce the bug discovered in hook version 1.1.2 where migration
398+
// tries to modify liquidity positions that have zero liquidity
399+
vm.createSelectFork("base", 35671635);
400+
401+
address contentCoin = 0x81f5F30217dA777a5d6441606AFa57E093833d7C;
402+
address oldHook = 0x9ea932730A7787000042e34390B8E435dD839040; // v1.1.2 hook
403+
address newHook = 0xff74Be9D3596eA7a33BB4983DD7906fB34135040; // current hook
404+
address upgradeGate = 0xD88f6BdD765313CaFA5888C177c325E2C3AbF2D2; // deployed upgrade gate
405+
406+
BaseCoin coin = BaseCoin(contentCoin);
407+
408+
// Register upgrade path
409+
address[] memory baseImpls = new address[](1);
410+
baseImpls[0] = oldHook;
411+
412+
vm.prank(Ownable(upgradeGate).owner());
413+
IHooksUpgradeGate(upgradeGate).registerUpgradePath(baseImpls, newHook);
414+
415+
// Get coin owner
416+
address coinOwner = MultiOwnable(contentCoin).owners()[0];
417+
418+
// First, demonstrate the bug exists - this should fail
419+
vm.prank(coinOwner);
420+
vm.expectRevert();
421+
coin.migrateLiquidity(newHook, "");
422+
423+
// Now fix the bug by etching fixed hook code onto the old hook address
424+
bytes memory creationCode = HooksDeployment.contentCoinCreationCode(address(poolManager), coinVersionLookup, new address[](0), upgradeGate);
425+
426+
(IHooks fixedHook, ) = HooksDeployment.deployHookWithExistingOrNewSalt(address(this), creationCode, bytes32(0));
427+
428+
// Etch the fixed hook code onto the old hook address
429+
vm.etch(oldHook, address(fixedHook).code);
430+
431+
// Now migration should work
432+
vm.prank(coinOwner);
433+
coin.migrateLiquidity(newHook, "");
434+
}
391435
}

0 commit comments

Comments
 (0)