feat: do not callback hook if caller is hook (#29)

* feat: do not callback hook if caller is hook

* chore: gas tweak

* test: update test after merge

.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap

@@ -1 +1 @@
-143699
+143903

.forge-snapshots/BinHookTest#testDonateSucceedsWithHook.snap

@@ -1 +1 @@
-135043
+135247

.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap

@@ -1 +1 @@
-108914
+109118

.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap

@@ -1 +1 @@
-301091
+301295

.forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap

@@ -1 +1 @@
-139136
+139320

.forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap

@@ -1 +1 @@
-90479
+90617

.forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap

@@ -1 +1 @@
-65672
+65810

.forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap

@@ -1 +1 @@
-149227
+149365

.forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap

@@ -1 +1 @@
-66851
+66989

.forge-snapshots/BinPoolManagerTest#testGasDonate.snap

@@ -1 +1 @@
-52444
+52582

.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap

@@ -1 +1 @@
-968444
+968582

.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap

@@ -1 +1 @@
-119958
+120096

.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap

@@ -1 +1 @@
-337634
+337772

.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap

@@ -1 +1 @@
-54695
+54833

.forge-snapshots/BinPoolManagerTest#testGasSwapMultipleBins.snap

@@ -1 +1 @@
-89312
+89370

.forge-snapshots/BinPoolManagerTest#testGasSwapOverBigBinIdGate.snap

@@ -1 +1 @@
-91297
+91355

.forge-snapshots/BinPoolManagerTest#testGasSwapSingleBin.snap

@@ -1 +1 @@
-70741
+70859

.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap

@@ -1 +1 @@
-319341
+319479

.forge-snapshots/BinPoolManagerTest#testNoOpGas_Burn.snap

@@ -1 +1 @@
-41503
+41726

.forge-snapshots/BinPoolManagerTest#testNoOpGas_Donate.snap

@@ -1 +1 @@
-19392
+19494

.forge-snapshots/BinPoolManagerTest#testNoOpGas_Initialize.snap

@@ -1 +1 @@
-37117
+37255

.forge-snapshots/BinPoolManagerTest#testNoOpGas_Mint.snap

@@ -1 +1 @@
-39136
+39359

.forge-snapshots/BinPoolManagerTest#testNoOpGas_Swap.snap

@@ -1 +1 @@
-22507
+22739

.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap

@@ -1 +1 @@
-349756
+349902

.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap

@@ -1 +1 @@
-60304
+60450

.forge-snapshots/CLPoolManagerTest#addLiquidity_nativeToken.snap

@@ -1 +1 @@
-243346
+243492

.forge-snapshots/CLPoolManagerTest#donateBothTokens.snap

@@ -1 +1 @@
-82481
+82627

.forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap

@@ -1 +1 @@
-52443
+52589

.forge-snapshots/CLPoolManagerTest#initializeWithoutHooks.snap

@@ -1 +1 @@
-36559
+36705

.forge-snapshots/CLPoolManagerTest#removeLiquidity_toNonEmpty.snap

@@ -1 +1 @@
-43238
+43384

.forge-snapshots/CLPoolManagerTest#swap_againstLiquidity.snap

@@ -1 +1 @@
-54615
+54761

.forge-snapshots/CLPoolManagerTest#swap_leaveSurplusTokenInVault.snap

@@ -1 +1 @@
-100694
+100840

.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap

@@ -1 +1 @@
-25040401
+25040547

.forge-snapshots/CLPoolManagerTest#swap_simple.snap

@@ -1 +1 @@
-35815
+35961

.forge-snapshots/CLPoolManagerTest#swap_useSurplusTokenAsInput.snap

@@ -1 +1 @@
-101452
+101598

.forge-snapshots/CLPoolManagerTest#swap_withHooks.snap

@@ -1 +1 @@
-41474
+41680

.forge-snapshots/CLPoolManagerTest#swap_withNative.snap

@@ -1 +1 @@
-35818
+35964

.forge-snapshots/CLPoolManagerTest#testNoOp_gas_Donate.snap

@@ -1 +1 @@
-19186
+19394

.forge-snapshots/CLPoolManagerTest#testNoOp_gas_Initialize.snap

@@ -1 +1 @@
-37517
+37663

.forge-snapshots/CLPoolManagerTest#testNoOp_gas_ModifyPosition.snap

@@ -1 +1 @@
-29901
+30109

.forge-snapshots/CLPoolManagerTest#testNoOp_gas_Swap.snap

@@ -1 +1 @@
-21613
+21824

src/libraries/Hooks.sol

@@ -45,14 +45,22 @@ library Hooks {
         }
     }
 
-    function shouldCall(bytes32 parameters, uint8 offset) internal pure returns (bool) {
+    /// @return true if parameter has offset enabled
+    function hasOffsetEnabled(bytes32 parameters, uint8 offset) internal pure returns (bool) {
         return parameters.decodeBool(offset);
     }
 
+    /// @notice checks if hook should be called -- based on 2 factors:
+    /// 1. whether pool.parameters has the callback offset registered
+    /// 2. whether msg.sender is the hook itself
+    function shouldCall(bytes32 parameters, uint8 offset, IHooks hook) internal view returns (bool) {
+        return hasOffsetEnabled(parameters, offset) && address(hook) != msg.sender;
+    }
+
     /// @dev Verify hook return value matches no-op when these 2 conditions are met
     ///   1) Hook have permission for no-op
     ///   2) Return value is no-op selector
     function isValidNoOpCall(bytes32 parameters, uint8 noOpOffset, bytes4 selector) internal pure returns (bool) {
-        return shouldCall(parameters, noOpOffset) && selector == NO_OP_SELECTOR;
+        return hasOffsetEnabled(parameters, noOpOffset) && selector == NO_OP_SELECTOR;
     }
 }

src/pool-bin/BinPoolManager.sol

@@ -106,7 +106,7 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         uint24 swapFee = key.fee.getInitialSwapFee();
         if (swapFee.isSwapFeeTooLarge(SwapFeeLibrary.TEN_PERCENT_FEE)) revert FeeTooLarge();
 
-        if (key.parameters.shouldCall(HOOKS_BEFORE_INITIALIZE_OFFSET)) {
+        if (key.parameters.shouldCall(HOOKS_BEFORE_INITIALIZE_OFFSET, hooks)) {
             if (hooks.beforeInitialize(msg.sender, key, activeId, hookData) != IBinHooks.beforeInitialize.selector) {
                 revert Hooks.InvalidHookResponse();
             }
@@ -120,7 +120,7 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         /// @notice Make sure the first event is noted, so that later events from afterHook won't get mixed up with this one
         emit Initialize(id, key.currency0, key.currency1, key.fee, binStep, hooks);
 
-        if (key.parameters.shouldCall(HOOKS_AFTER_INITIALIZE_OFFSET)) {
+        if (key.parameters.shouldCall(HOOKS_AFTER_INITIALIZE_OFFSET, hooks)) {
             if (hooks.afterInitialize(msg.sender, key, activeId, hookData) != IBinHooks.afterInitialize.selector) {
                 revert Hooks.InvalidHookResponse();
             }
@@ -139,7 +139,7 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         _checkPoolInitialized(id);
 
         IBinHooks hooks = IBinHooks(address(key.hooks));
-        if (key.parameters.shouldCall(HOOKS_BEFORE_SWAP_OFFSET)) {
+        if (key.parameters.shouldCall(HOOKS_BEFORE_SWAP_OFFSET, hooks)) {
             bytes4 selector = hooks.beforeSwap(msg.sender, key, swapForY, amountIn, hookData);
             if (key.parameters.isValidNoOpCall(HOOKS_NO_OP_OFFSET, selector)) {
                 // Sentinel return value used to signify that a NoOp occurred.
@@ -170,7 +170,7 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
             emit Swap(id, msg.sender, delta.amount0(), delta.amount1(), activeId, swapFee, feeForProtocol);
         }
 
-        if (key.parameters.shouldCall(HOOKS_AFTER_SWAP_OFFSET)) {
+        if (key.parameters.shouldCall(HOOKS_AFTER_SWAP_OFFSET, hooks)) {
             if (hooks.afterSwap(msg.sender, key, swapForY, amountIn, delta, hookData) != IBinHooks.afterSwap.selector) {
                 revert Hooks.InvalidHookResponse();
             }
@@ -241,7 +241,7 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         _checkPoolInitialized(id);
 
         IBinHooks hooks = IBinHooks(address(key.hooks));
-        if (key.parameters.shouldCall(HOOKS_BEFORE_MINT_OFFSET)) {
+        if (key.parameters.shouldCall(HOOKS_BEFORE_MINT_OFFSET, hooks)) {
             bytes4 selector = hooks.beforeMint(msg.sender, key, params, hookData);
             if (key.parameters.isValidNoOpCall(HOOKS_NO_OP_OFFSET, selector)) {
                 // Sentinel return value used to signify that a NoOp occurred.
@@ -274,7 +274,7 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         /// @notice Make sure the first event is noted, so that later events from afterHook won't get mixed up with this one
         emit Mint(id, msg.sender, mintArray.ids, mintArray.amounts, compositionFee, feeForProtocol);
 
-        if (key.parameters.shouldCall(HOOKS_AFTER_MINT_OFFSET)) {
+        if (key.parameters.shouldCall(HOOKS_AFTER_MINT_OFFSET, hooks)) {
             if (hooks.afterMint(msg.sender, key, params, delta, hookData) != IBinHooks.afterMint.selector) {
                 revert Hooks.InvalidHookResponse();
             }
@@ -292,7 +292,7 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         _checkPoolInitialized(id);
 
         IBinHooks hooks = IBinHooks(address(key.hooks));
-        if (key.parameters.shouldCall(HOOKS_BEFORE_BURN_OFFSET)) {
+        if (key.parameters.shouldCall(HOOKS_BEFORE_BURN_OFFSET, hooks)) {
             bytes4 selector = hooks.beforeBurn(msg.sender, key, params, hookData);
             if (key.parameters.isValidNoOpCall(HOOKS_NO_OP_OFFSET, selector)) {
                 // Sentinel return value used to signify that a NoOp occurred.
@@ -312,7 +312,7 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         /// @notice Make sure the first event is noted, so that later events from afterHook won't get mixed up with this one
         emit Burn(id, msg.sender, binIds, amountRemoved);
 
-        if (key.parameters.shouldCall(HOOKS_AFTER_BURN_OFFSET)) {
+        if (key.parameters.shouldCall(HOOKS_AFTER_BURN_OFFSET, hooks)) {
             if (hooks.afterBurn(msg.sender, key, params, delta, hookData) != IBinHooks.afterBurn.selector) {
                 revert Hooks.InvalidHookResponse();
             }
@@ -330,7 +330,7 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         _checkPoolInitialized(id);
 
         IBinHooks hooks = IBinHooks(address(key.hooks));
-        if (key.parameters.shouldCall(HOOKS_BEFORE_DONATE_OFFSET)) {
+        if (key.parameters.shouldCall(HOOKS_BEFORE_DONATE_OFFSET, hooks)) {
             bytes4 selector = hooks.beforeDonate(msg.sender, key, amount0, amount1, hookData);
             if (key.parameters.isValidNoOpCall(HOOKS_NO_OP_OFFSET, selector)) {
                 // Sentinel return value used to signify that a NoOp occurred.
@@ -347,7 +347,7 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         /// @notice Make sure the first event is noted, so that later events from afterHook won't get mixed up with this one
         emit Donate(id, msg.sender, delta.amount0(), delta.amount1(), binId);
 
-        if (key.parameters.shouldCall(HOOKS_AFTER_DONATE_OFFSET)) {
+        if (key.parameters.shouldCall(HOOKS_AFTER_DONATE_OFFSET, hooks)) {
             if (hooks.afterDonate(msg.sender, key, amount0, amount1, hookData) != IBinHooks.afterDonate.selector) {
                 revert Hooks.InvalidHookResponse();
             }
@@ -386,12 +386,12 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
 
     function _validateHookNoOp(PoolKey memory key) internal pure {
         // if no-op is active for hook, there must be a before* hook active too
-        if (key.parameters.shouldCall(HOOKS_NO_OP_OFFSET)) {
+        if (key.parameters.hasOffsetEnabled(HOOKS_NO_OP_OFFSET)) {
             if (
-                !key.parameters.shouldCall(HOOKS_BEFORE_MINT_OFFSET)
-                    && !key.parameters.shouldCall(HOOKS_BEFORE_BURN_OFFSET)
-                    && !key.parameters.shouldCall(HOOKS_BEFORE_SWAP_OFFSET)
-                    && !key.parameters.shouldCall(HOOKS_BEFORE_DONATE_OFFSET)
+                !key.parameters.hasOffsetEnabled(HOOKS_BEFORE_MINT_OFFSET)
+                    && !key.parameters.hasOffsetEnabled(HOOKS_BEFORE_BURN_OFFSET)
+                    && !key.parameters.hasOffsetEnabled(HOOKS_BEFORE_SWAP_OFFSET)
+                    && !key.parameters.hasOffsetEnabled(HOOKS_BEFORE_DONATE_OFFSET)
             ) {
                 revert Hooks.NoOpHookMissingBeforeCall();
             }