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

skip calls to hooks when msg.sender is hook contract + test #503

Merged
merged 19 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity with empty hook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
312159
312221
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity with native token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
192338
192320
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
192319
192301

This file was deleted.

1 change: 0 additions & 1 deletion .forge-snapshots/cached dynamic fee, no hooks.snap

This file was deleted.

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 @@
132057
132039
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 @@
177151
177133
1 change: 0 additions & 1 deletion .forge-snapshots/mintWithEmptyHookEOAInitiated.snap

This file was deleted.

1 change: 0 additions & 1 deletion .forge-snapshots/modify position with noop.snap

This file was deleted.

2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
22928
22916
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity with empty hook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
98936
98918
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity with native token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
200309
200291
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
196586
196568
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 @@
187218
187059
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
195758
195599
1 change: 0 additions & 1 deletion .forge-snapshots/simpleSwapEOAInitiated.snap

This file was deleted.

1 change: 0 additions & 1 deletion .forge-snapshots/simpleSwapNativeEOAInitiated.snap

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1 +1 @@
117877
117718
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
105344
105185
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn 6909 for input.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
125493
125334
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn native 6909 for input.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
121446
121287
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint native output as 6909.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
189776
189617
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint output as 6909.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
206585
206426
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 @@
138649
138490
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
105322
105163
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 @@
189651
189532
93 changes: 47 additions & 46 deletions src/libraries/Hooks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -53,20 +53,20 @@ library Hooks {
/// the deployed hooks address causes the intended hooks to be called
/// @param permissions The hooks that are intended to be called
/// @dev permissions param is memory as the function will be called from constructors
function validateHookPermissions(IHooks self, Permissions memory permissions) internal pure {
function validateHookPermissions(IHooks hook, Permissions memory permissions) internal pure {
if (
permissions.beforeInitialize != self.hasPermission(BEFORE_INITIALIZE_FLAG)
|| permissions.afterInitialize != self.hasPermission(AFTER_INITIALIZE_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)
|| permissions.afterDonate != self.hasPermission(AFTER_DONATE_FLAG)
permissions.beforeInitialize != hook.hasPermission(BEFORE_INITIALIZE_FLAG)
|| permissions.afterInitialize != hook.hasPermission(AFTER_INITIALIZE_FLAG)
|| permissions.beforeAddLiquidity != hook.hasPermission(BEFORE_ADD_LIQUIDITY_FLAG)
|| permissions.afterAddLiquidity != hook.hasPermission(AFTER_ADD_LIQUIDITY_FLAG)
|| permissions.beforeRemoveLiquidity != hook.hasPermission(BEFORE_REMOVE_LIQUIDITY_FLAG)
|| permissions.afterRemoveLiquidity != hook.hasPermission(AFTER_REMOVE_LIQUIDITY_FLAG)
|| permissions.beforeSwap != hook.hasPermission(BEFORE_SWAP_FLAG)
|| permissions.afterSwap != hook.hasPermission(AFTER_SWAP_FLAG)
|| permissions.beforeDonate != hook.hasPermission(BEFORE_DONATE_FLAG)
|| permissions.afterDonate != hook.hasPermission(AFTER_DONATE_FLAG)
) {
revert HookAddressNotValid(address(self));
revert HookAddressNotValid(address(hook));
}
}

Expand All @@ -83,129 +83,130 @@ library Hooks {
/// @notice performs a hook call using the given calldata on the given hook
/// @return expectedSelector The selector that the hook is expected to return
/// @return selector The selector that the hook actually returned
function _callHook(IHooks self, bytes memory data) private returns (bytes4 expectedSelector, bytes4 selector) {
function _callHook(IHooks hook, bytes memory data) private returns (bytes4 expectedSelector, bytes4 selector) {
assembly {
expectedSelector := mload(add(data, 0x20))
}

(bool success, bytes memory result) = address(self).call(data);
(bool success, bytes memory result) = address(hook).call(data);
if (!success) _revert(result);

selector = abi.decode(result, (bytes4));
}

/// @notice performs a hook call using the given calldata on the given hook
function callHook(IHooks self, bytes memory data) internal {
(bytes4 expectedSelector, bytes4 selector) = _callHook(self, data);
function callHook(IHooks hook, bytes memory data) internal {
if (msg.sender == address(hook)) return;
(bytes4 expectedSelector, bytes4 selector) = _callHook(hook, data);

if (selector != expectedSelector) {
revert InvalidHookResponse();
}
}

/// @notice calls beforeInitialize hook if permissioned and validates return value
function beforeInitialize(IHooks self, PoolKey memory key, uint160 sqrtPriceX96, bytes calldata hookData)
function beforeInitialize(IHooks hook, PoolKey memory key, uint160 sqrtPriceX96, bytes calldata hookData)
internal
{
if (self.hasPermission(BEFORE_INITIALIZE_FLAG)) {
self.callHook(
if (hook.hasPermission(BEFORE_INITIALIZE_FLAG)) {
hook.callHook(
abi.encodeWithSelector(IHooks.beforeInitialize.selector, msg.sender, key, sqrtPriceX96, hookData)
);
}
}

/// @notice calls afterInitialize hook if permissioned and validates return value
function afterInitialize(IHooks self, PoolKey memory key, uint160 sqrtPriceX96, int24 tick, bytes calldata hookData)
function afterInitialize(IHooks hook, PoolKey memory key, uint160 sqrtPriceX96, int24 tick, bytes calldata hookData)
internal
{
if (self.hasPermission(AFTER_INITIALIZE_FLAG)) {
self.callHook(
if (hook.hasPermission(AFTER_INITIALIZE_FLAG)) {
hook.callHook(
abi.encodeWithSelector(IHooks.afterInitialize.selector, msg.sender, key, sqrtPriceX96, tick, hookData)
);
}
}

/// @notice calls beforeModifyLiquidity hook if permissioned and validates return value
function beforeModifyLiquidity(
IHooks self,
IHooks hook,
PoolKey memory key,
IPoolManager.ModifyLiquidityParams memory params,
bytes calldata hookData
) internal {
if (params.liquidityDelta > 0 && key.hooks.hasPermission(BEFORE_ADD_LIQUIDITY_FLAG)) {
self.callHook(abi.encodeWithSelector(IHooks.beforeAddLiquidity.selector, msg.sender, key, params, hookData));
} else if (params.liquidityDelta <= 0 && key.hooks.hasPermission(BEFORE_REMOVE_LIQUIDITY_FLAG)) {
self.callHook(
if (params.liquidityDelta > 0 && hook.hasPermission(BEFORE_ADD_LIQUIDITY_FLAG)) {
hook.callHook(abi.encodeWithSelector(IHooks.beforeAddLiquidity.selector, msg.sender, key, params, hookData));
} else if (params.liquidityDelta <= 0 && hook.hasPermission(BEFORE_REMOVE_LIQUIDITY_FLAG)) {
hook.callHook(
abi.encodeWithSelector(IHooks.beforeRemoveLiquidity.selector, msg.sender, key, params, hookData)
);
}
}

/// @notice calls afterModifyLiquidity hook if permissioned and validates return value
function afterModifyLiquidity(
IHooks self,
IHooks hook,
PoolKey memory key,
IPoolManager.ModifyLiquidityParams memory params,
BalanceDelta delta,
bytes calldata hookData
) internal {
if (params.liquidityDelta > 0 && key.hooks.hasPermission(AFTER_ADD_LIQUIDITY_FLAG)) {
self.callHook(
if (params.liquidityDelta > 0 && hook.hasPermission(AFTER_ADD_LIQUIDITY_FLAG)) {
hook.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(
} else if (params.liquidityDelta <= 0 && hook.hasPermission(AFTER_REMOVE_LIQUIDITY_FLAG)) {
hook.callHook(
abi.encodeWithSelector(IHooks.afterRemoveLiquidity.selector, msg.sender, key, params, delta, hookData)
);
}
}

/// @notice calls beforeSwap hook if permissioned and validates return value
function beforeSwap(IHooks self, PoolKey memory key, IPoolManager.SwapParams memory params, bytes calldata hookData)
function beforeSwap(IHooks hook, PoolKey memory key, IPoolManager.SwapParams memory params, bytes calldata hookData)
internal
{
if (key.hooks.hasPermission(BEFORE_SWAP_FLAG)) {
self.callHook(abi.encodeWithSelector(IHooks.beforeSwap.selector, msg.sender, key, params, hookData));
if (hook.hasPermission(BEFORE_SWAP_FLAG)) {
hook.callHook(abi.encodeWithSelector(IHooks.beforeSwap.selector, msg.sender, key, params, hookData));
}
}

/// @notice calls afterSwap hook if permissioned and validates return value
function afterSwap(
IHooks self,
IHooks hook,
PoolKey memory key,
IPoolManager.SwapParams memory params,
BalanceDelta delta,
bytes calldata hookData
) internal {
if (key.hooks.hasPermission(AFTER_SWAP_FLAG)) {
self.callHook(abi.encodeWithSelector(IHooks.afterSwap.selector, msg.sender, key, params, delta, hookData));
if (hook.hasPermission(AFTER_SWAP_FLAG)) {
hook.callHook(abi.encodeWithSelector(IHooks.afterSwap.selector, msg.sender, key, params, delta, hookData));
}
}

/// @notice calls beforeDonate hook if permissioned and validates return value
function beforeDonate(IHooks self, PoolKey memory key, uint256 amount0, uint256 amount1, bytes calldata hookData)
function beforeDonate(IHooks hook, PoolKey memory key, uint256 amount0, uint256 amount1, bytes calldata hookData)
internal
{
if (key.hooks.hasPermission(BEFORE_DONATE_FLAG)) {
self.callHook(
if (hook.hasPermission(BEFORE_DONATE_FLAG)) {
hook.callHook(
abi.encodeWithSelector(IHooks.beforeDonate.selector, msg.sender, key, amount0, amount1, hookData)
);
}
}

/// @notice calls afterDonate hook if permissioned and validates return value
function afterDonate(IHooks self, PoolKey memory key, uint256 amount0, uint256 amount1, bytes calldata hookData)
function afterDonate(IHooks hook, PoolKey memory key, uint256 amount0, uint256 amount1, bytes calldata hookData)
internal
{
if (key.hooks.hasPermission(AFTER_DONATE_FLAG)) {
self.callHook(
if (hook.hasPermission(AFTER_DONATE_FLAG)) {
hook.callHook(
abi.encodeWithSelector(IHooks.afterDonate.selector, msg.sender, key, amount0, amount1, hookData)
);
}
}

function hasPermission(IHooks self, uint256 flag) internal pure returns (bool) {
return uint256(uint160(address(self))) & flag != 0;
function hasPermission(IHooks hook, uint256 flag) internal pure returns (bool) {
return uint256(uint160(address(hook))) & flag != 0;
}

/// @notice bubble up revert if present. Else throw FailedHookCall
Expand Down
13 changes: 4 additions & 9 deletions src/test/PoolSwapTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,16 @@ contract PoolSwapTest is Test, PoolTestBase {

CallbackData memory data = abi.decode(rawData, (CallbackData));

(,, uint256 reserveBefore0, int256 deltaBefore0) =
_fetchBalances(data.key.currency0, data.sender, address(this));
(,, uint256 reserveBefore1, int256 deltaBefore1) =
_fetchBalances(data.key.currency1, data.sender, address(this));
(,,, int256 deltaBefore0) = _fetchBalances(data.key.currency0, data.sender, address(this));
(,,, int256 deltaBefore1) = _fetchBalances(data.key.currency1, data.sender, address(this));

assertEq(deltaBefore0, 0);
assertEq(deltaBefore1, 0);

BalanceDelta delta = manager.swap(data.key, data.params, data.hookData);

(,, uint256 reserveAfter0, int256 deltaAfter0) = _fetchBalances(data.key.currency0, data.sender, address(this));
(,, uint256 reserveAfter1, int256 deltaAfter1) = _fetchBalances(data.key.currency1, data.sender, address(this));

assertEq(reserveBefore0, reserveAfter0);
Copy link
Member

Choose a reason for hiding this comment

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

why do we remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There could be a swap within the hook making the reserves not equal to each other so I removed it. Also since reservesOf might be removed entirely anyways

assertEq(reserveBefore1, reserveAfter1);
(,,, int256 deltaAfter0) = _fetchBalances(data.key.currency0, data.sender, address(this));
(,,, int256 deltaAfter1) = _fetchBalances(data.key.currency1, data.sender, address(this));

if (data.params.zeroForOne) {
if (data.params.amountSpecified < 0) {
Expand Down
49 changes: 49 additions & 0 deletions src/test/SkipCallsTestHook.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.24;

import {Hooks} from "../libraries/Hooks.sol";
import {BaseTestHooks} from "./BaseTestHooks.sol";
import {IHooks} from "../interfaces/IHooks.sol";
import {IPoolManager} from "../interfaces/IPoolManager.sol";
import {PoolKey} from "../types/PoolKey.sol";
import {BalanceDelta} from "../types/BalanceDelta.sol";
import {PoolId, PoolIdLibrary} from "../types/PoolId.sol";
import {IERC20Minimal} from "../interfaces/external/IERC20Minimal.sol";
import {CurrencyLibrary, Currency} from "../types/Currency.sol";
import {PoolTestBase} from "./PoolTestBase.sol";
import {Test} from "forge-std/Test.sol";

contract SkipCallsTestHook is BaseTestHooks, Test {
using PoolIdLibrary for PoolKey;
using Hooks for IHooks;

uint256 public counter;
IPoolManager manager;
uint24 internal fee;

function setManager(IPoolManager _manager) external {
manager = _manager;
}

function beforeSwap(address, PoolKey calldata key, IPoolManager.SwapParams calldata params, bytes calldata hookData)
external
override
returns (bytes4)
{
counter++;
callSwap(key, params, hookData);
return IHooks.beforeSwap.selector;
}

function callSwap(PoolKey calldata key, IPoolManager.SwapParams calldata params, bytes calldata hookData) public {
IPoolManager(manager).swap(key, params, hookData);
address payer = abi.decode(hookData, (address));
int256 delta0 = IPoolManager(manager).currencyDelta(address(this), key.currency0);
assertEq(delta0, params.amountSpecified);
int256 delta1 = IPoolManager(manager).currencyDelta(address(this), key.currency1);
assert(delta1 > 0);
IERC20Minimal(Currency.unwrap(key.currency0)).transferFrom(payer, address(manager), uint256(-delta0));
manager.settle(key.currency0);
manager.take(key.currency1, payer, uint256(delta1));
}
}
Loading
Loading