Skip to content

Commit

Permalink
refactor: hooks callsites (Uniswap#439)
Browse files Browse the repository at this point in the history
* feat: hooks refactor

* fix: tests

* feat: noop -> shouldExecute

* fix: remove shouldCall functions for helper

* fix: using for in test

* fix: alice comment
  • Loading branch information
marktoda authored and hyunchel committed Feb 21, 2024
1 parent 8277dc6 commit 6d5987f
Show file tree
Hide file tree
Showing 34 changed files with 390 additions and 333 deletions.
2 changes: 1 addition & 1 deletion .forge-snapshots/HooksShouldCallBeforeSwap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
34
116
Original file line number Diff line number Diff line change
@@ -1 +1 @@
192918
194519
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 @@
148280
146864
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 @@
139280
137968
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 @@
186702
185390
2 changes: 1 addition & 1 deletion .forge-snapshots/initialize.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
75775
74268
2 changes: 1 addition & 1 deletion .forge-snapshots/mint with empty hook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
318670
323262
2 changes: 1 addition & 1 deletion .forge-snapshots/mint with native token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
201474
200100
2 changes: 1 addition & 1 deletion .forge-snapshots/mint.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
201394
200042
2 changes: 1 addition & 1 deletion .forge-snapshots/mintWithEmptyHookEOAInitiated.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
250566
255158
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 @@
56582
58201
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
24030
23592
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 @@
197165
195835
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
205733
204403
2 changes: 1 addition & 1 deletion .forge-snapshots/simpleSwapEOAInitiated.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
176999
175669
2 changes: 1 addition & 1 deletion .forge-snapshots/simpleSwapNativeEOAInitiated.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
173661
172331
Original file line number Diff line number Diff line change
@@ -1 +1 @@
127793
126463
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
115284
113954
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 @@
134457
133127
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 @@
217926
216596
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 @@
192160
193620
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
115262
113932
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with noop.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
49888
51546
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 @@
198762
200363
2 changes: 1 addition & 1 deletion lib/forge-gas-snapshot
95 changes: 12 additions & 83 deletions src/PoolManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {

function _checkLocker(address caller, address locker, IHooks hook) internal pure {
if (caller == locker) return;
if (caller == address(hook) && hook.hasPermissionToAccessLock()) return;
if (caller == address(hook) && hook.hasPermission(Hooks.ACCESS_LOCK_FLAG)) return;
revert LockedBy(locker, address(hook));
}

Expand All @@ -119,32 +119,15 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {
if (key.currency0 >= key.currency1) revert CurrenciesOutOfOrderOrEqual();
if (!key.hooks.isValidHookAddress(key.fee)) revert Hooks.HookAddressNotValid(address(key.hooks));

(bool set) = Lockers.setCurrentHook(key.hooks);

if (key.hooks.shouldCallBeforeInitialize()) {
if (key.hooks.beforeInitialize(msg.sender, key, sqrtPriceX96, hookData) != IHooks.beforeInitialize.selector)
{
revert Hooks.InvalidHookResponse();
}
}
key.hooks.beforeInitialize(key, sqrtPriceX96, hookData);

PoolId id = key.toId();
(, uint16 protocolFee) = _fetchProtocolFee(key);
uint24 swapFee = key.fee.isDynamicFee() ? _fetchDynamicSwapFee(key) : key.fee.getStaticFee();

tick = pools[id].initialize(sqrtPriceX96, protocolFee, swapFee);

if (key.hooks.shouldCallAfterInitialize()) {
if (
key.hooks.afterInitialize(msg.sender, key, sqrtPriceX96, tick, hookData)
!= IHooks.afterInitialize.selector
) {
revert Hooks.InvalidHookResponse();
}
}

// We only want to clear the current hook if it was set in setCurrentHook in this execution frame.
if (set) Lockers.clearCurrentHook();
key.hooks.afterInitialize(key, sqrtPriceX96, tick, hookData);

// On intitalize we emit the key's fee, which tells us all fee settings a pool can have: either a static swap fee or dynamic swap fee and if the hook has enabled swap or withdraw fees.
emit Initialize(id, key.currency0, key.currency1, key.fee, key.tickSpacing, key.hooks);
Expand Down Expand Up @@ -199,21 +182,11 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {
IPoolManager.ModifyPositionParams memory params,
bytes calldata hookData
) external override noDelegateCall onlyByLocker returns (BalanceDelta delta) {
(bool set) = Lockers.setCurrentHook(key.hooks);

PoolId id = key.toId();
_checkPoolInitialized(id);

if (key.hooks.shouldCallBeforeModifyPosition()) {
bytes4 selector = key.hooks.beforeModifyPosition(msg.sender, key, params, hookData);
// Sentinel return value used to signify that a NoOp occurred.
if (key.hooks.isValidNoOpCall(selector)) {
// We only want to clear the current hook if it was set in setCurrentHook in this execution frame.
if (set) Lockers.clearCurrentHook();
return BalanceDeltaLibrary.MAXIMUM_DELTA;
} else if (selector != IHooks.beforeModifyPosition.selector) {
revert Hooks.InvalidHookResponse();
}
if (!key.hooks.beforeModifyPosition(key, params, hookData)) {
return BalanceDeltaLibrary.MAXIMUM_DELTA;
}

delta = pools[id].modifyPosition(
Expand All @@ -228,17 +201,7 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {

_accountPoolBalanceDelta(key, delta);

if (key.hooks.shouldCallAfterModifyPosition()) {
if (
key.hooks.afterModifyPosition(msg.sender, key, params, delta, hookData)
!= IHooks.afterModifyPosition.selector
) {
revert Hooks.InvalidHookResponse();
}
}

// We only want to clear the current hook if it was set in setCurrentHook in this execution frame.
if (set) Lockers.clearCurrentHook();
key.hooks.afterModifyPosition(key, params, delta, hookData);

emit ModifyPosition(id, msg.sender, params.tickLower, params.tickUpper, params.liquidityDelta);
}
Expand All @@ -251,21 +214,11 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {
onlyByLocker
returns (BalanceDelta delta)
{
(bool set) = Lockers.setCurrentHook(key.hooks);

PoolId id = key.toId();
_checkPoolInitialized(id);

if (key.hooks.shouldCallBeforeSwap()) {
bytes4 selector = key.hooks.beforeSwap(msg.sender, key, params, hookData);
// Sentinel return value used to signify that a NoOp occurred.
if (key.hooks.isValidNoOpCall(selector)) {
// We only want to clear the current hook if it was set in setCurrentHook in this execution frame.
if (set) Lockers.clearCurrentHook();
return BalanceDeltaLibrary.MAXIMUM_DELTA;
} else if (selector != IHooks.beforeSwap.selector) {
revert Hooks.InvalidHookResponse();
}
if (!key.hooks.beforeSwap(key, params, hookData)) {
return BalanceDeltaLibrary.MAXIMUM_DELTA;
}

uint256 feeForProtocol;
Expand All @@ -289,14 +242,7 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {
}
}

if (key.hooks.shouldCallAfterSwap()) {
if (key.hooks.afterSwap(msg.sender, key, params, delta, hookData) != IHooks.afterSwap.selector) {
revert Hooks.InvalidHookResponse();
}
}

// We only want to clear the current hook if it was set in setCurrentHook in this execution frame.
if (set) Lockers.clearCurrentHook();
key.hooks.afterSwap(key, params, delta, hookData);

emit Swap(
id, msg.sender, delta.amount0(), delta.amount1(), state.sqrtPriceX96, state.liquidity, state.tick, swapFee
Expand All @@ -311,35 +257,18 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {
onlyByLocker
returns (BalanceDelta delta)
{
(bool set) = Lockers.setCurrentHook(key.hooks);

PoolId id = key.toId();
_checkPoolInitialized(id);

if (key.hooks.shouldCallBeforeDonate()) {
bytes4 selector = key.hooks.beforeDonate(msg.sender, key, amount0, amount1, hookData);
// Sentinel return value used to signify that a NoOp occurred.
if (key.hooks.isValidNoOpCall(selector)) {
// We only want to clear the current hook if it was set in setCurrentHook in this execution frame.
if (set) Lockers.clearCurrentHook();
return BalanceDeltaLibrary.MAXIMUM_DELTA;
} else if (selector != IHooks.beforeDonate.selector) {
revert Hooks.InvalidHookResponse();
}
if (!key.hooks.beforeDonate(key, amount0, amount1, hookData)) {
return BalanceDeltaLibrary.MAXIMUM_DELTA;
}

delta = pools[id].donate(amount0, amount1);

_accountPoolBalanceDelta(key, delta);

if (key.hooks.shouldCallAfterDonate()) {
if (key.hooks.afterDonate(msg.sender, key, amount0, amount1, hookData) != IHooks.afterDonate.selector) {
revert Hooks.InvalidHookResponse();
}
}

// We only want to clear the current hook if it was set in setCurrentHook in this execution frame.
if (set) Lockers.clearCurrentHook();
key.hooks.afterDonate(key, amount0, amount1, hookData);
}

/// @inheritdoc IPoolManager
Expand Down
Loading

0 comments on commit 6d5987f

Please sign in to comment.