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.

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

Before after add/remove liquidity, remove all modifyPosition hooks #434

Merged
merged 31 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
8bf9f1b
Rip out beforeModifyPosition and after, add mints
zhongeric Nov 13, 2023
5babed0
forge fmt
zhongeric Nov 13, 2023
bb11369
Add explicit before/after mint hook calls
zhongeric Nov 13, 2023
de292c1
Add mintPosition
zhongeric Nov 14, 2023
c6a223c
fix wrong test assumption
zhongeric Nov 15, 2023
38a1e5d
forge fmt
zhongeric Nov 15, 2023
c7e4a72
natspec
zhongeric Nov 15, 2023
b24c159
Merge branch 'main' into before-after-mint
zhongeric Nov 17, 2023
90af549
chore: change mintPosition -> addLiquidity
rileydcampbell Nov 22, 2023
97779f5
merged changes
rileydcampbell Nov 24, 2023
b0fea97
fix tests
rileydcampbell Nov 27, 2023
25314e3
Merge branch 'main' of https://github.com/Uniswap/v4-core into before…
rileydcampbell Dec 6, 2023
3f4467d
Merge branch 'main' of https://github.com/Uniswap/v4-core into before…
rileydcampbell Dec 6, 2023
1a75990
More tests
rileydcampbell Dec 6, 2023
33e602d
More tests
rileydcampbell Dec 6, 2023
21514b4
Merge branch 'before-after-mint' of https://github.com/Uniswap/v4-cor…
rileydcampbell Dec 6, 2023
518737a
Change modify position params for PoolManager
rileydcampbell Dec 7, 2023
3fd4079
Merge branch 'main' of https://github.com/Uniswap/v4-core into before…
rileydcampbell Dec 7, 2023
cee4d82
Fixed PoolModifyPositionTest and some more breaking tests.
rileydcampbell Dec 7, 2023
0d37920
New gas snapshots
rileydcampbell Dec 7, 2023
17b439a
fix test comments
rileydcampbell Dec 7, 2023
689fab4
Merge branch 'main' of https://github.com/Uniswap/v4-core into before…
rileydcampbell Dec 8, 2023
399f250
fix: use bound instead of vm.assume
rileydcampbell Dec 8, 2023
6c497bc
pull in changes and refactor hook calls
rileydcampbell Dec 12, 2023
832e6b1
Merge branch 'main' of https://github.com/Uniswap/v4-core into before…
rileydcampbell Dec 12, 2023
435c914
fix tests
rileydcampbell Dec 12, 2023
389789c
addressed feedback
rileydcampbell Dec 12, 2023
8c5e457
Change entry point name and no hooks on liquidity delta zero
rileydcampbell Dec 13, 2023
1260a27
call before/afterRemove Liquidity hooks on zero liquidity delta
rileydcampbell Dec 13, 2023
43d5aea
fix naming
rileydcampbell Dec 14, 2023
e408681
change event name
rileydcampbell Dec 14, 2023
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
1 change: 1 addition & 0 deletions .forge-snapshots/addLiquidity with empty hook.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
323249
1 change: 1 addition & 0 deletions .forge-snapshots/addLiquidity with native token.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
200141
1 change: 1 addition & 0 deletions .forge-snapshots/addLiquidity.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
200109
Original file line number Diff line number Diff line change
@@ -1 +1 @@
194519
194300
2 changes: 1 addition & 1 deletion .forge-snapshots/cached dynamic fee, no hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
146864
146667
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 1 token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
137968
137771
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 2 tokens.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
185390
185216
2 changes: 1 addition & 1 deletion .forge-snapshots/erc20 collect protocol fees.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
26968
27033
2 changes: 1 addition & 1 deletion .forge-snapshots/gas overhead of no-op lock.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
15246
15224
2 changes: 1 addition & 1 deletion .forge-snapshots/initialize.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
74268
74200
2 changes: 1 addition & 1 deletion .forge-snapshots/mintWithEmptyHookEOAInitiated.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
255158
255129
2 changes: 1 addition & 1 deletion .forge-snapshots/modify position with noop.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
58201
58013
2 changes: 1 addition & 1 deletion .forge-snapshots/native collect protocol fees.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
38634
38699
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
23592
23823
1 change: 1 addition & 0 deletions .forge-snapshots/removeLiquidity with empty hook.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
106962
1 change: 1 addition & 0 deletions .forge-snapshots/removeLiquidity with native token.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
208322
1 change: 1 addition & 0 deletions .forge-snapshots/removeLiquidity.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
204612
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap with native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
195835
195638
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
204403
204206
2 changes: 1 addition & 1 deletion .forge-snapshots/simpleSwapEOAInitiated.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
175669
175475
2 changes: 1 addition & 1 deletion .forge-snapshots/simpleSwapNativeEOAInitiated.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
172331
172131
Original file line number Diff line number Diff line change
@@ -1 +1 @@
126463
126266
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
113954
113757
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn claim for input.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
133127
132930
1 change: 1 addition & 0 deletions .forge-snapshots/swap mint 1155 as output.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
169598
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint output as claim.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
216596
216399
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with dynamic fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
193620
193401
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
113932
113735
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with noop.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
51546
51304
2 changes: 1 addition & 1 deletion .forge-snapshots/update dynamic fee in before swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
200363
200144
2 changes: 1 addition & 1 deletion lib/forge-gas-snapshot
2 changes: 1 addition & 1 deletion src/PoolManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {
}

/// @inheritdoc IPoolManager
function modifyPosition(
function modifyLiquidity(
PoolKey memory key,
IPoolManager.ModifyPositionParams memory params,
bytes calldata hookData
Expand Down
43 changes: 35 additions & 8 deletions src/interfaces/IHooks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,53 @@ interface IHooks {
bytes calldata hookData
) external returns (bytes4);

/// @notice The hook called before a position is modified
/// @param sender The initial msg.sender for the modify position call
/// @notice The hook called before liquidity is added
/// @param sender The initial msg.sender for the add liquidity call
/// @param key The key for the pool
/// @param params The parameters for modifying the position
/// @param params The parameters for adding liquidity
/// @param hookData Arbitrary data handed into the PoolManager by the liquidty provider to be be passed on to the hook
/// @return bytes4 The function selector for the hook
function beforeModifyPosition(
function beforeAddLiquidity(
address sender,
PoolKey calldata key,
IPoolManager.ModifyPositionParams calldata params,
bytes calldata hookData
) external returns (bytes4);

/// @notice The hook called after a position is modified
/// @param sender The initial msg.sender for the modify position call
/// @notice The hook called after liquidity is added
/// @param sender The initial msg.sender for the add liquidity call
/// @param key The key for the pool
/// @param params The parameters for modifying the position
/// @param params The parameters for adding liquidity
/// @param hookData Arbitrary data handed into the PoolManager by the liquidty provider to be be passed on to the hook
/// @return bytes4 The function selector for the hook
function afterModifyPosition(
function afterAddLiquidity(
address sender,
PoolKey calldata key,
IPoolManager.ModifyPositionParams calldata params,
BalanceDelta delta,
bytes calldata hookData
) external returns (bytes4);

/// @notice The hook called before liquidity is removed
/// @param sender The initial msg.sender for the remove liquidity call
/// @param key The key for the pool
/// @param params The parameters for removing liquidity
/// @param hookData Arbitrary data handed into the PoolManager by the liquidty provider to be be passed on to the hook
/// @return bytes4 The function selector for the hook
function beforeRemoveLiquidity(
address sender,
PoolKey calldata key,
IPoolManager.ModifyPositionParams calldata params,
bytes calldata hookData
) external returns (bytes4);

/// @notice The hook called after liquidity is removed
/// @param sender The initial msg.sender for the remove liquidity call
/// @param key The key for the pool
/// @param params The parameters for removing liquidity
/// @param hookData Arbitrary data handed into the PoolManager by the liquidty provider to be be passed on to the hook
/// @return bytes4 The function selector for the hook
function afterRemoveLiquidity(
address sender,
PoolKey calldata key,
IPoolManager.ModifyPositionParams calldata params,
Expand Down
9 changes: 7 additions & 2 deletions src/interfaces/IPoolManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,13 @@ interface IPoolManager is IFees, IClaims {
int256 liquidityDelta;
}

/// @notice Modify the position for the given pool
function modifyPosition(PoolKey memory key, ModifyPositionParams memory params, bytes calldata hookData)
/// @notice Modify the liquidity for the given pool
/// @dev Poke by calling with a zero liquidityDelta
/// @param key The pool to modify liquidity in
/// @param params The parameters for modifying the liquidity
/// @param hookData Any data to pass to the callback, via `ILockCallback(msg.sender).lockAcquired(data)`
/// @return delta The balance delta of the liquidity
function modifyLiquidity(PoolKey memory key, ModifyPositionParams memory params, bytes calldata hookData)
Copy link
Member

Choose a reason for hiding this comment

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

laaaast lil nit: ModifyLiquidityParams ?

external
returns (BalanceDelta);

Expand Down
57 changes: 36 additions & 21 deletions src/libraries/Hooks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,33 @@ import {Lockers} from "./Lockers.sol";
/// @notice V4 decides whether to invoke specific hooks by inspecting the leading bits of the address that
/// the hooks contract is deployed to.
/// For example, a hooks contract deployed to address: 0x9000000000000000000000000000000000000000
/// has leading bits '1001' which would cause the 'before initialize' and 'after modify position' hooks to be used.
/// has leading bits '1001' which would cause the 'before initialize' and 'after add liquidity' hooks to be used.
library Hooks {
using FeeLibrary for uint24;
using Hooks for IHooks;

uint256 internal constant BEFORE_INITIALIZE_FLAG = 1 << 159;
uint256 internal constant AFTER_INITIALIZE_FLAG = 1 << 158;
uint256 internal constant BEFORE_MODIFY_POSITION_FLAG = 1 << 157;
uint256 internal constant AFTER_MODIFY_POSITION_FLAG = 1 << 156;
uint256 internal constant BEFORE_SWAP_FLAG = 1 << 155;
uint256 internal constant AFTER_SWAP_FLAG = 1 << 154;
uint256 internal constant BEFORE_DONATE_FLAG = 1 << 153;
uint256 internal constant AFTER_DONATE_FLAG = 1 << 152;
uint256 internal constant NO_OP_FLAG = 1 << 151;
uint256 internal constant ACCESS_LOCK_FLAG = 1 << 150;
uint256 internal constant BEFORE_ADD_LIQUIDITY_FLAG = 1 << 157;
uint256 internal constant AFTER_ADD_LIQUIDITY_FLAG = 1 << 156;
uint256 internal constant BEFORE_REMOVE_LIQUIDITY_FLAG = 1 << 155;
uint256 internal constant AFTER_REMOVE_LIQUIDITY_FLAG = 1 << 154;
uint256 internal constant BEFORE_SWAP_FLAG = 1 << 153;
uint256 internal constant AFTER_SWAP_FLAG = 1 << 152;
uint256 internal constant BEFORE_DONATE_FLAG = 1 << 151;
uint256 internal constant AFTER_DONATE_FLAG = 1 << 150;
uint256 internal constant NO_OP_FLAG = 1 << 149;
uint256 internal constant ACCESS_LOCK_FLAG = 1 << 148;

bytes4 public constant NO_OP_SELECTOR = bytes4(keccak256(abi.encodePacked("NoOp")));

struct Permissions {
bool beforeInitialize;
bool afterInitialize;
bool beforeModifyPosition;
bool afterModifyPosition;
bool beforeAddLiquidity;
bool afterAddLiquidity;
bool beforeRemoveLiquidity;
bool afterRemoveLiquidity;
bool beforeSwap;
bool afterSwap;
bool beforeDonate;
Expand All @@ -60,8 +64,10 @@ library Hooks {
if (
permissions.beforeInitialize != self.hasPermission(BEFORE_INITIALIZE_FLAG)
|| permissions.afterInitialize != self.hasPermission(AFTER_INITIALIZE_FLAG)
|| permissions.beforeModifyPosition != self.hasPermission(BEFORE_MODIFY_POSITION_FLAG)
|| permissions.afterModifyPosition != self.hasPermission(AFTER_MODIFY_POSITION_FLAG)
|| permissions.beforeAddLiquidity != self.hasPermission(BEFORE_ADD_LIQUIDITY_FLAG)
|| permissions.afterAddLiquidity != self.hasPermission(AFTER_ADD_LIQUIDITY_FLAG)
|| permissions.beforeRemoveLiquidity != self.hasPermission(BEFORE_REMOVE_LIQUIDITY_FLAG)
|| permissions.afterRemoveLiquidity != self.hasPermission(AFTER_REMOVE_LIQUIDITY_FLAG)
|| permissions.beforeSwap != self.hasPermission(BEFORE_SWAP_FLAG)
|| permissions.afterSwap != self.hasPermission(AFTER_SWAP_FLAG)
|| permissions.beforeDonate != self.hasPermission(BEFORE_DONATE_FLAG)
Expand All @@ -76,10 +82,11 @@ library Hooks {
/// @notice Ensures that the hook address includes at least one hook flag or dynamic fees, or is the 0 address
/// @param hook The hook to verify
function isValidHookAddress(IHooks hook, uint24 fee) internal pure returns (bool) {
// if NoOp is allowed, at least one of beforeModifyPosition, beforeSwap and beforeDonate should be allowed
// if NoOp is allowed, at least one of beforeRemoveLiquidity, beforeAddLiquidity, beforeSwap and beforeDonate should be allowed
if (
hook.hasPermission(NO_OP_FLAG) && !hook.hasPermission(BEFORE_MODIFY_POSITION_FLAG)
&& !hook.hasPermission(BEFORE_SWAP_FLAG) && !hook.hasPermission(BEFORE_DONATE_FLAG)
hook.hasPermission(NO_OP_FLAG) && !hook.hasPermission(BEFORE_ADD_LIQUIDITY_FLAG)
&& !hook.hasPermission(BEFORE_REMOVE_LIQUIDITY_FLAG) && !hook.hasPermission(BEFORE_SWAP_FLAG)
&& !hook.hasPermission(BEFORE_DONATE_FLAG)
) {
return false;
}
Expand Down Expand Up @@ -161,12 +168,16 @@ library Hooks {
IPoolManager.ModifyPositionParams memory params,
bytes calldata hookData
) internal returns (bool shouldExecute) {
Copy link
Member

Choose a reason for hiding this comment

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

and same name change on these functions? Just thinking so that they are all standardized

beforeModifyPosition -> beforeModifyLiquidity
afterModifyPosition -> afterModifyLiquidity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed 👍

if (self.hasPermission(BEFORE_MODIFY_POSITION_FLAG)) {
if (params.liquidityDelta > 0 && key.hooks.hasPermission(BEFORE_ADD_LIQUIDITY_FLAG)) {
shouldExecute = self.callHookNoopable(
abi.encodeWithSelector(IHooks.beforeModifyPosition.selector, msg.sender, key, params, hookData)
abi.encodeWithSelector(IHooks.beforeAddLiquidity.selector, msg.sender, key, params, hookData)
);
} else if (params.liquidityDelta <= 0 && key.hooks.hasPermission(BEFORE_REMOVE_LIQUIDITY_FLAG)) {
shouldExecute = self.callHookNoopable(
abi.encodeWithSelector(IHooks.beforeRemoveLiquidity.selector, msg.sender, key, params, hookData)
);
} else {
return true;
shouldExecute = true;
}
}

Expand All @@ -178,9 +189,13 @@ library Hooks {
BalanceDelta delta,
bytes calldata hookData
) internal {
if (key.hooks.hasPermission(AFTER_MODIFY_POSITION_FLAG)) {
if (params.liquidityDelta > 0 && key.hooks.hasPermission(AFTER_ADD_LIQUIDITY_FLAG)) {
self.callHook(
abi.encodeWithSelector(IHooks.afterAddLiquidity.selector, msg.sender, key, params, delta, hookData)
);
} else if (params.liquidityDelta <= 0 && key.hooks.hasPermission(AFTER_REMOVE_LIQUIDITY_FLAG)) {
self.callHook(
abi.encodeWithSelector(IHooks.afterModifyPosition.selector, msg.sender, key, params, delta, hookData)
abi.encodeWithSelector(IHooks.afterRemoveLiquidity.selector, msg.sender, key, params, delta, hookData)
);
}
}
Expand Down
Loading