Skip to content

Commit

Permalink
skip calls to hooks when msg.sender is hook contract + test (Uniswap#503
Browse files Browse the repository at this point in the history
)

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

* move into callHook

* changes

* linting

* re-add forge snapshots

* switch back to self instead of hook

* more tests

* call again and assert counter is 2

* make more clear

* clearAllHookPermisssionsMask

* add comments and switch back to uint256 for now

* allowance does not get decremented if the sender address is the contract address

* fix claims

* remove import console.sol
  • Loading branch information
dianakocsis authored Apr 5, 2024
1 parent 65235ed commit 8a4ebb0
Show file tree
Hide file tree
Showing 26 changed files with 472 additions and 55 deletions.
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 @@
265311
265373
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 @@
140154
140136
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
145471
145453
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 @@
101145
101049
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 @@
132135
132039
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
22974
22962
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 @@
56002
55984
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 @@
148188
148170
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
149652
149634
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 @@
132905
132814
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
146781
146690
Original file line number Diff line number Diff line change
@@ -1 +1 @@
72364
72273
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
60367
60276
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 @@
80397
80306
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 @@
76394
76303
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 @@
138745
138654
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 @@
155596
155505
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 @@
89671
89580
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
60345
60254
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 @@
140297
140246
25 changes: 13 additions & 12 deletions src/libraries/Hooks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ 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) {
/// @param self The hook to verify
function isValidHookAddress(IHooks self, uint24 fee) internal pure returns (bool) {
// If there is no hook contract set, then fee cannot be dynamic
// If a hook contract is set, it must have at least 1 flag set, or have a dynamic fee
return address(hook) == address(0)
return address(self) == address(0)
? !fee.isDynamicFee()
: (uint160(address(hook)) >= AFTER_DONATE_FLAG || fee.isDynamicFee());
: (uint160(address(self)) >= AFTER_DONATE_FLAG || fee.isDynamicFee());
}

/// @notice performs a hook call using the given calldata on the given hook
Expand All @@ -96,6 +96,7 @@ library Hooks {

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

if (selector != expectedSelector) {
Expand Down Expand Up @@ -132,9 +133,9 @@ library Hooks {
IPoolManager.ModifyLiquidityParams memory params,
bytes calldata hookData
) internal {
if (params.liquidityDelta > 0 && key.hooks.hasPermission(BEFORE_ADD_LIQUIDITY_FLAG)) {
if (params.liquidityDelta > 0 && self.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)) {
} else if (params.liquidityDelta <= 0 && self.hasPermission(BEFORE_REMOVE_LIQUIDITY_FLAG)) {
self.callHook(
abi.encodeWithSelector(IHooks.beforeRemoveLiquidity.selector, msg.sender, key, params, hookData)
);
Expand All @@ -149,11 +150,11 @@ library Hooks {
BalanceDelta delta,
bytes calldata hookData
) internal {
if (params.liquidityDelta > 0 && key.hooks.hasPermission(AFTER_ADD_LIQUIDITY_FLAG)) {
if (params.liquidityDelta > 0 && self.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)) {
} else if (params.liquidityDelta <= 0 && self.hasPermission(AFTER_REMOVE_LIQUIDITY_FLAG)) {
self.callHook(
abi.encodeWithSelector(IHooks.afterRemoveLiquidity.selector, msg.sender, key, params, delta, hookData)
);
Expand All @@ -164,7 +165,7 @@ library Hooks {
function beforeSwap(IHooks self, PoolKey memory key, IPoolManager.SwapParams memory params, bytes calldata hookData)
internal
{
if (key.hooks.hasPermission(BEFORE_SWAP_FLAG)) {
if (self.hasPermission(BEFORE_SWAP_FLAG)) {
self.callHook(abi.encodeWithSelector(IHooks.beforeSwap.selector, msg.sender, key, params, hookData));
}
}
Expand All @@ -177,7 +178,7 @@ library Hooks {
BalanceDelta delta,
bytes calldata hookData
) internal {
if (key.hooks.hasPermission(AFTER_SWAP_FLAG)) {
if (self.hasPermission(AFTER_SWAP_FLAG)) {
self.callHook(abi.encodeWithSelector(IHooks.afterSwap.selector, msg.sender, key, params, delta, hookData));
}
}
Expand All @@ -186,7 +187,7 @@ library Hooks {
function beforeDonate(IHooks self, PoolKey memory key, uint256 amount0, uint256 amount1, bytes calldata hookData)
internal
{
if (key.hooks.hasPermission(BEFORE_DONATE_FLAG)) {
if (self.hasPermission(BEFORE_DONATE_FLAG)) {
self.callHook(
abi.encodeWithSelector(IHooks.beforeDonate.selector, msg.sender, key, amount0, amount1, hookData)
);
Expand All @@ -197,7 +198,7 @@ library Hooks {
function afterDonate(IHooks self, PoolKey memory key, uint256 amount0, uint256 amount1, bytes calldata hookData)
internal
{
if (key.hooks.hasPermission(AFTER_DONATE_FLAG)) {
if (self.hasPermission(AFTER_DONATE_FLAG)) {
self.callHook(
abi.encodeWithSelector(IHooks.afterDonate.selector, msg.sender, key, amount0, amount1, hookData)
);
Expand Down
14 changes: 4 additions & 10 deletions src/test/PoolDonateTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,17 @@ contract PoolDonateTest is PoolTestBase {

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

(, uint256 poolBalanceBefore0, int256 deltaBefore0) =
_fetchBalances(data.key.currency0, data.sender, address(this));
(, uint256 poolBalanceBefore1, 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));

require(deltaBefore0 == 0, "deltaBefore0 is not 0");
require(deltaBefore1 == 0, "deltaBefore1 is not 0");

BalanceDelta delta = manager.donate(data.key, data.amount0, data.amount1, data.hookData);

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

require(poolBalanceBefore0 == poolBalanceAfter0, "poolBalanceBefore0 is not equal to poolBalanceAfter0");
require(poolBalanceBefore1 == poolBalanceAfter1, "poolBalanceBefore1 is not equal to poolBalanceAfter1");
require(deltaAfter0 == -int256(data.amount0), "deltaAfter0 is not equal to -int256(data.amount0)");
require(deltaAfter1 == -int256(data.amount1), "deltaAfter1 is not equal to -int256(data.amount1)");

Expand Down
15 changes: 4 additions & 11 deletions src/test/PoolSwapTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,23 +52,16 @@ contract PoolSwapTest is PoolTestBase {

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

(, uint256 poolBalanceBefore0, int256 deltaBefore0) =
_fetchBalances(data.key.currency0, data.sender, address(this));
(, uint256 poolBalanceBefore1, 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));

require(deltaBefore0 == 0, "deltaBefore0 is not equal to 0");
require(deltaBefore1 == 0, "deltaBefore1 is not equal to 0");

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

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

require(poolBalanceBefore0 == poolBalanceAfter0, "poolBalanceBefore0 is not equal to poolBalanceAfter0");
require(poolBalanceBefore1 == poolBalanceAfter1, "poolBalanceBefore1 is not equal to poolBalanceAfter1");
(,, 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
Loading

0 comments on commit 8a4ebb0

Please sign in to comment.