Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Test lockless posm via hooks #266

Merged
merged 6 commits into from
Aug 4, 2024
Merged

Test lockless posm via hooks #266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f034243
test hook which modifiesLiquidities in beforeSwap
saucepoint Aug 4, 2024
15588ce
test hook modifying liquidity
saucepoint Aug 4, 2024
87b9da0
minor cleanups
saucepoint Aug 4, 2024
1dd5349
test that hooks cannot re-enter modifyLiquidities
saucepoint Aug 4, 2024
ea69949
hook mints liquidity with modifyLiquidities
saucepoint Aug 4, 2024
7743e05
Merge branch 'actions-no-unlock' into sauce/actions-no-unlock-hook
saucepoint Aug 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
294 changes: 294 additions & 0 deletions test/position-managers/PositionManager.modifiyLiquidities.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,294 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;

import "forge-std/Test.sol";
import {PoolManager} from "@uniswap/v4-core/src/PoolManager.sol";
import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
import {IHooks} from "@uniswap/v4-core/src/interfaces/IHooks.sol";
import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol";
import {Currency, CurrencyLibrary} from "@uniswap/v4-core/src/types/Currency.sol";
import {PoolId, PoolIdLibrary} from "@uniswap/v4-core/src/types/PoolId.sol";
import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";
import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol";
import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol";
import {Position} from "@uniswap/v4-core/src/libraries/Position.sol";
import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol";

import {IPositionManager} from "../../src/interfaces/IPositionManager.sol";
import {ReentrancyLock} from "../../src/base/ReentrancyLock.sol";
import {Actions} from "../../src/libraries/Actions.sol";
import {PositionManager} from "../../src/PositionManager.sol";
import {PositionConfig} from "../../src/libraries/PositionConfig.sol";

import {LiquidityFuzzers} from "../shared/fuzz/LiquidityFuzzers.sol";
import {Planner, Plan} from "../shared/Planner.sol";
import {PosmTestSetup} from "../shared/PosmTestSetup.sol";

contract PositionManagerModifyLiquiditiesTest is Test, PosmTestSetup, LiquidityFuzzers {
using StateLibrary for IPoolManager;
using PoolIdLibrary for PoolKey;

PoolId poolId;
address alice;
uint256 alicePK;
address bob;

PositionConfig config;

function setUp() public {
(alice, alicePK) = makeAddrAndKey("ALICE");
(bob,) = makeAddrAndKey("BOB");

deployFreshManagerAndRouters();
deployMintAndApprove2Currencies();

// Requires currency0 and currency1 to be set in base Deployers contract.
deployAndApprovePosm(manager);

seedBalance(alice);
approvePosmFor(alice);

// must deploy after posm
// Deploys a hook which can accesses IPositionManager.modifyLiquidities
deployPosmHookModifyLiquidities();
seedBalance(address(hookModifyLiquidities));

(key, poolId) = initPool(currency0, currency1, IHooks(hookModifyLiquidities), 3000, SQRT_PRICE_1_1, ZERO_BYTES);

config = PositionConfig({poolKey: key, tickLower: -60, tickUpper: 60});
}

/// @dev minting liquidity without approval is allowable
function test_hook_mint() public {
uint256 initialLiquidity = 100e18;
uint256 tokenId = lpm.nextTokenId();
mint(config, initialLiquidity, address(this), ZERO_BYTES);

// hook mints a new position in beforeSwap via hookData
uint256 hookTokenId = lpm.nextTokenId();
uint256 newLiquidity = 10e18;
bytes memory calls = getMintEncoded(config, newLiquidity, address(hookModifyLiquidities), ZERO_BYTES);

swap(key, true, -1e18, calls);

bytes32 positionId =
Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId));
(uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId);

// original liquidity unchanged
assertEq(liquidity, initialLiquidity);

// hook minted its own position
positionId =
Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(hookTokenId));
(liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId);
assertEq(liquidity, newLiquidity);

assertEq(lpm.ownerOf(tokenId), address(this)); // original position owned by this contract
assertEq(lpm.ownerOf(hookTokenId), address(hookModifyLiquidities)); // hook position owned by hook
}

/// @dev increasing liquidity without approval is allowable
function test_hook_increaseLiquidity() public {
saucepoint marked this conversation as resolved.
Show resolved Hide resolved
uint256 initialLiquidity = 100e18;
uint256 tokenId = lpm.nextTokenId();
mint(config, initialLiquidity, address(this), ZERO_BYTES);

// hook increases liquidity in beforeSwap via hookData
uint256 newLiquidity = 10e18;
bytes memory calls = getIncreaseEncoded(tokenId, config, newLiquidity, ZERO_BYTES);

swap(key, true, -1e18, calls);

bytes32 positionId =
Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId));
(uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId);

assertEq(liquidity, initialLiquidity + newLiquidity);
}

/// @dev hook can decrease liquidity with approval
function test_hook_decreaseLiquidity() public {
uint256 initialLiquidity = 100e18;
uint256 tokenId = lpm.nextTokenId();
mint(config, initialLiquidity, address(this), ZERO_BYTES);

// approve the hook for decreasing liquidity
lpm.approve(address(hookModifyLiquidities), tokenId);

// hook decreases liquidity in beforeSwap via hookData
uint256 liquidityToDecrease = 10e18;
bytes memory calls = getDecreaseEncoded(tokenId, config, liquidityToDecrease, ZERO_BYTES);

swap(key, true, -1e18, calls);

bytes32 positionId =
Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId));
(uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId);

assertEq(liquidity, initialLiquidity - liquidityToDecrease);
}

/// @dev hook can collect liquidity with approval
function test_hook_collect() public {
uint256 initialLiquidity = 100e18;
uint256 tokenId = lpm.nextTokenId();
mint(config, initialLiquidity, address(this), ZERO_BYTES);

// approve the hook for collecting liquidity
lpm.approve(address(hookModifyLiquidities), tokenId);

// donate to generate revenue
uint256 feeRevenue0 = 1e18;
uint256 feeRevenue1 = 0.1e18;
donateRouter.donate(config.poolKey, feeRevenue0, feeRevenue1, ZERO_BYTES);

uint256 balance0HookBefore = currency0.balanceOf(address(hookModifyLiquidities));
uint256 balance1HookBefore = currency1.balanceOf(address(hookModifyLiquidities));

// hook collects liquidity in beforeSwap via hookData
bytes memory calls = getCollectEncoded(tokenId, config, ZERO_BYTES);
swap(key, true, -1e18, calls);

bytes32 positionId =
Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId));
(uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId);

// liquidity unchanged
assertEq(liquidity, initialLiquidity);

// hook collected the fee revenue
assertEq(currency0.balanceOf(address(hookModifyLiquidities)), balance0HookBefore + feeRevenue0 - 1 wei); // imprecision, core is keeping 1 wei
assertEq(currency1.balanceOf(address(hookModifyLiquidities)), balance1HookBefore + feeRevenue1 - 1 wei);
}

/// @dev hook can burn liquidity with approval
function test_hook_burn() public {
// mint some liquidity that is NOT burned in beforeSwap
mint(config, 100e18, address(this), ZERO_BYTES);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whats the point in this mint that doesnt get burned?

Copy link
Collaborator Author

@saucepoint saucepoint Aug 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because we're burning a position in beforeSwap, if there is only 1 position the swap will fail haha (all liquidity was burned JIT)

need some "idle" liquidity thats untouched for the swap to succeed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL good catch


// the position to be burned by the hook
uint256 initialLiquidity = 100e18;
uint256 tokenId = lpm.nextTokenId();
mint(config, initialLiquidity, address(this), ZERO_BYTES);
// TODO: make this less jank since HookModifyLiquidites also has delta saving capabilities
// BalanceDelta mintDelta = getLastDelta();
BalanceDelta mintDelta = hookModifyLiquidities.deltas(hookModifyLiquidities.numberDeltasReturned() - 1);

// approve the hook for burning liquidity
lpm.approve(address(hookModifyLiquidities), tokenId);

uint256 balance0HookBefore = currency0.balanceOf(address(hookModifyLiquidities));
uint256 balance1HookBefore = currency1.balanceOf(address(hookModifyLiquidities));

// hook burns liquidity in beforeSwap via hookData
bytes memory calls = getBurnEncoded(tokenId, config, ZERO_BYTES);
swap(key, true, -1e18, calls);

bytes32 positionId =
Position.calculatePositionKey(address(lpm), config.tickLower, config.tickUpper, bytes32(tokenId));
(uint256 liquidity,,) = manager.getPositionInfo(config.poolKey.toId(), positionId);

// liquidity burned
assertEq(liquidity, 0);
// 721 will revert if the token does not exist
vm.expectRevert();
lpm.ownerOf(tokenId);

// hook claimed the burned liquidity
assertEq(
currency0.balanceOf(address(hookModifyLiquidities)),
balance0HookBefore + uint128(-mintDelta.amount0() - 1 wei) // imprecision since core is keeping 1 wei
);
assertEq(
currency1.balanceOf(address(hookModifyLiquidities)),
balance1HookBefore + uint128(-mintDelta.amount1() - 1 wei)
);
}

// --- Revert Scenarios --- //
/// @dev Hook does not have approval so decreasingly liquidity should revert
function test_hook_decreaseLiquidity_revert() public {
uint256 initialLiquidity = 100e18;
uint256 tokenId = lpm.nextTokenId();
mint(config, initialLiquidity, address(this), ZERO_BYTES);

// hook decreases liquidity in beforeSwap via hookData
uint256 liquidityToDecrease = 10e18;
bytes memory calls = getDecreaseEncoded(tokenId, config, liquidityToDecrease, ZERO_BYTES);

// should revert because hook is not approved
vm.expectRevert(
abi.encodeWithSelector(
Hooks.FailedHookCall.selector,
abi.encodeWithSelector(IPositionManager.NotApproved.selector, address(hookModifyLiquidities))
)
);
swap(key, true, -1e18, calls);
}

/// @dev hook does not have approval so collecting liquidity should revert
function test_hook_collect_revert() public {
uint256 initialLiquidity = 100e18;
uint256 tokenId = lpm.nextTokenId();
mint(config, initialLiquidity, address(this), ZERO_BYTES);

// donate to generate revenue
uint256 feeRevenue0 = 1e18;
uint256 feeRevenue1 = 0.1e18;
donateRouter.donate(config.poolKey, feeRevenue0, feeRevenue1, ZERO_BYTES);

// hook collects liquidity in beforeSwap via hookData
bytes memory calls = getCollectEncoded(tokenId, config, ZERO_BYTES);

// should revert because hook is not approved
vm.expectRevert(
abi.encodeWithSelector(
Hooks.FailedHookCall.selector,
abi.encodeWithSelector(IPositionManager.NotApproved.selector, address(hookModifyLiquidities))
)
);
swap(key, true, -1e18, calls);
}

/// @dev hook does not have approval so burning liquidity should revert
function test_hook_burn_revert() public {
// the position to be burned by the hook
uint256 initialLiquidity = 100e18;
uint256 tokenId = lpm.nextTokenId();
mint(config, initialLiquidity, address(this), ZERO_BYTES);

// hook burns liquidity in beforeSwap via hookData
bytes memory calls = getBurnEncoded(tokenId, config, ZERO_BYTES);

// should revert because hook is not approved
vm.expectRevert(
abi.encodeWithSelector(
Hooks.FailedHookCall.selector,
abi.encodeWithSelector(IPositionManager.NotApproved.selector, address(hookModifyLiquidities))
hensha256 marked this conversation as resolved.
Show resolved Hide resolved
)
);
swap(key, true, -1e18, calls);
}

/// @dev hook cannot re-enter modifyLiquidities in beforeRemoveLiquidity
function test_hook_increaseLiquidity_reenter_revert() public {
uint256 initialLiquidity = 100e18;
uint256 tokenId = lpm.nextTokenId();
mint(config, initialLiquidity, address(this), ZERO_BYTES);

uint256 newLiquidity = 10e18;

// to be provided as hookData, so beforeAddLiquidity attempts to increase liquidity
bytes memory hookCall = getIncreaseEncoded(tokenId, config, newLiquidity, ZERO_BYTES);
bytes memory calls = getIncreaseEncoded(tokenId, config, newLiquidity, hookCall);

// should revert because hook is re-entering modifyLiquidities
vm.expectRevert(
abi.encodeWithSelector(
Hooks.FailedHookCall.selector, abi.encodeWithSelector(ReentrancyLock.ContractLocked.selector)
)
);
lpm.unlockAndModifyLiquidities(calls, _deadline);
}
}
2 changes: 1 addition & 1 deletion test/position-managers/PositionManager.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers {
assertEq(currency0.balanceOfSelf(), balance0BeforeBurn + uint256(int256(deltaDecrease.amount0())));
assertEq(currency1.balanceOfSelf(), balance1BeforeBurn + uint256(uint128(deltaDecrease.amount1())));

// OZ 721 will revert if the token does not exist
hensha256 marked this conversation as resolved.
Show resolved Hide resolved
// 721 will revert if the token does not exist
vm.expectRevert();
lpm.ownerOf(1);

Expand Down
75 changes: 75 additions & 0 deletions test/shared/HookModifyLiquidities.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.24;

import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol";

import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
import {Currency} from "@uniswap/v4-core/src/types/Currency.sol";
import {BeforeSwapDelta, BeforeSwapDeltaLibrary} from "@uniswap/v4-core/src/types/BeforeSwapDelta.sol";
import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";
import {BalanceDelta, BalanceDeltaLibrary} from "@uniswap/v4-core/src/types/BalanceDelta.sol";

import {HookSavesDelta} from "./HookSavesDelta.sol";
import {IERC20} from "forge-std/interfaces/IERC20.sol";

import {IPositionManager} from "../../src/interfaces/IPositionManager.sol";

/// @notice This contract is NOT a production use contract. It is meant to be used in testing to verify that external contracts can modify liquidity without a lock (IPositionManager.modifyLiquidities)
/// @dev a hook that can modify liquidity in beforeSwap
contract HookModifyLiquidities is HookSavesDelta {
IPositionManager posm;
IAllowanceTransfer permit2;

function setAddresses(IPositionManager _posm, IAllowanceTransfer _permit2) external {
posm = _posm;
permit2 = _permit2;
}

function beforeSwap(
address, /* sender **/
PoolKey calldata key, /* key **/
IPoolManager.SwapParams calldata, /* params **/
bytes calldata hookData
) external override returns (bytes4, BeforeSwapDelta, uint24) {
approvePosmCurrency(key.currency0);
approvePosmCurrency(key.currency1);

(bytes memory actions, bytes[] memory params) = abi.decode(hookData, (bytes, bytes[]));
posm.modifyLiquidities(actions, params);
return (this.beforeSwap.selector, BeforeSwapDeltaLibrary.ZERO_DELTA, 0);
}

function beforeAddLiquidity(
address, /* sender **/
PoolKey calldata, /* key **/
IPoolManager.ModifyLiquidityParams calldata, /* params **/
bytes calldata hookData
) external override returns (bytes4) {
if (hookData.length > 0) {
(bytes memory actions, bytes[] memory params) = abi.decode(hookData, (bytes, bytes[]));
posm.modifyLiquidities(actions, params);
}
return this.beforeAddLiquidity.selector;
}

function beforeRemoveLiquidity(
address, /* sender **/
PoolKey calldata, /* key **/
IPoolManager.ModifyLiquidityParams calldata, /* params **/
bytes calldata hookData
) external override returns (bytes4) {
if (hookData.length > 0) {
(bytes memory actions, bytes[] memory params) = abi.decode(hookData, (bytes, bytes[]));
posm.modifyLiquidities(actions, params);
}
return this.beforeRemoveLiquidity.selector;
}

function approvePosmCurrency(Currency currency) internal {
// Because POSM uses permit2, we must execute 2 permits/approvals.
// 1. First, the caller must approve permit2 on the token.
IERC20(Currency.unwrap(currency)).approve(address(permit2), type(uint256).max);
saucepoint marked this conversation as resolved.
Show resolved Hide resolved
// 2. Then, the caller must approve POSM as a spender of permit2. TODO: This could also be a signature.
permit2.approve(Currency.unwrap(currency), address(posm), type(uint160).max, type(uint48).max);
}
}
Loading
Loading