Fix plugin hook recursion

docs/Contracts/Core/base/AlgebraPoolBase.md

@@ -267,7 +267,7 @@ function getCommunityFeePending() external view returns (uint128, uint128)
 
 The amounts of token0 and token1 that will be sent to the vault
 
-*Developer note: Will be sent COMMUNITY_FEE_TRANSFER_FREQUENCY after communityFeeLastTimestamp*
+*Developer note: Will be sent FEE_TRANSFER_FREQUENCY after communityFeeLastTimestamp*
 
 **Returns:**
 

docs/Contracts/Core/interfaces/pool/IAlgebraPoolState.md

@@ -129,7 +129,7 @@ function getCommunityFeePending() external view returns (uint128 communityFeePen
 
 The amounts of token0 and token1 that will be sent to the vault
 
-*Developer note: Will be sent COMMUNITY_FEE_TRANSFER_FREQUENCY after communityFeeLastTimestamp*
+*Developer note: Will be sent FEE_TRANSFER_FREQUENCY after communityFeeLastTimestamp*
 
 **Returns:**
 

src/core/contracts/AlgebraFactory.sol

@@ -57,7 +57,7 @@ contract AlgebraFactory is IAlgebraFactory, Ownable2Step, AccessControlEnumerabl
 
   /// @inheritdoc IAlgebraFactory
   /// @dev keccak256 of AlgebraPool init bytecode. Used to compute pool address deterministically
-  bytes32 public constant POOL_INIT_CODE_HASH = 0x4b9e4a8044ce5695e06fce9421a63b6f5c3db8a561eebb30ea4c775469e36eaf;
+  bytes32 public constant POOL_INIT_CODE_HASH = 0x53a254b73c7f4f4a23175de0908ad4b30f3bc60806bd69bba905db6f24b991a5;
 
   constructor(address _poolDeployer) {
     require(_poolDeployer != address(0));

src/core/contracts/AlgebraPool.sol

@@ -115,7 +115,7 @@ contract AlgebraPool is AlgebraPoolBase, TickStructure, ReentrancyGuard, Positio
       }
     }
 
-    _changeReserves(int256(amount0), int256(amount1), 0, 0);
+    _changeReserves(int256(amount0), int256(amount1), 0, 0, 0, 0);
     emit Mint(msg.sender, recipient, bottomTick, topTick, liquidityActual, amount0, amount1);
 
     _unlock();
@@ -133,18 +133,33 @@ contract AlgebraPool is AlgebraPoolBase, TickStructure, ReentrancyGuard, Positio
 
     int128 liquidityDelta = -int128(amount);
 
-    _beforeModifyPos(msg.sender, bottomTick, topTick, liquidityDelta, data);
+    uint24 pluginFee = _beforeModifyPos(msg.sender, bottomTick, topTick, liquidityDelta, data);
     _lock();
 
     _updateReserves();
-    Position storage position = getOrCreatePosition(msg.sender, bottomTick, topTick);
+    {
+      Position storage position = getOrCreatePosition(msg.sender, bottomTick, topTick);
 
-    (amount0, amount1) = _updatePositionTicksAndFees(position, bottomTick, topTick, liquidityDelta);
+      (amount0, amount1) = _updatePositionTicksAndFees(position, bottomTick, topTick, liquidityDelta);
 
-    if (amount0 | amount1 != 0) {
-      // since we do not support tokens whose total supply can exceed uint128, these casts are safe
-      // and, theoretically, unchecked cast prevents a complete blocking of burn
-      (position.fees0, position.fees1) = (position.fees0 + uint128(amount0), position.fees1 + uint128(amount1));
+      if (pluginFee > 0) {
+        if (amount0 > 0) {
+          uint256 deltaPluginFeePending0 = FullMath.mulDiv(amount0, pluginFee, Constants.FEE_DENOMINATOR);
+          amount0 -= deltaPluginFeePending0;
+          pluginFeePending0 += uint104(deltaPluginFeePending0);
+        }
+        if (amount1 > 0) {
+          uint256 deltaPluginFeePending1 = FullMath.mulDiv(amount1, pluginFee, Constants.FEE_DENOMINATOR);
+          amount1 -= deltaPluginFeePending1;
+          pluginFeePending1 += uint104(deltaPluginFeePending1);
+        }
+      }
+
+      if (amount0 | amount1 != 0) {
+        // since we do not support tokens whose total supply can exceed uint128, these casts are safe
+        // and, theoretically, unchecked cast prevents a complete blocking of burn
+        (position.fees0, position.fees1) = (position.fees0 + uint128(amount0), position.fees1 + uint128(amount1));
+      }
     }
 
     if (amount | amount0 | amount1 != 0) emit Burn(msg.sender, bottomTick, topTick, amount, amount0, amount1);
@@ -153,15 +168,28 @@ contract AlgebraPool is AlgebraPoolBase, TickStructure, ReentrancyGuard, Positio
     _afterModifyPos(msg.sender, bottomTick, topTick, liquidityDelta, amount0, amount1, data);
   }
 
-  function _beforeModifyPos(address owner, int24 bottomTick, int24 topTick, int128 liquidityDelta, bytes calldata data) internal {
+  function _isPlugin() internal view returns (bool) {
+    return msg.sender == plugin;
+  }
+
+  function _beforeModifyPos(
+    address owner,
+    int24 bottomTick,
+    int24 topTick,
+    int128 liquidityDelta,
+    bytes calldata data
+  ) internal returns (uint24 pluginFee) {
     if (globalState.pluginConfig.hasFlag(Plugins.BEFORE_POSITION_MODIFY_FLAG)) {
-      IAlgebraPlugin(plugin).beforeModifyPosition(msg.sender, owner, bottomTick, topTick, liquidityDelta, data).shouldReturn(
-        IAlgebraPlugin.beforeModifyPosition.selector
-      );
+      if (_isPlugin()) return 0;
+      bytes4 selector;
+      (selector, pluginFee) = IAlgebraPlugin(plugin).beforeModifyPosition(msg.sender, owner, bottomTick, topTick, liquidityDelta, data);
+      if (pluginFee >= 1e6) revert incorrectPluginFee();
+      selector.shouldReturn(IAlgebraPlugin.beforeModifyPosition.selector);
     }
   }
 
   function _afterModifyPos(address owner, int24 bTick, int24 tTick, int128 deltaL, uint256 amount0, uint256 amount1, bytes calldata data) internal {
+    if (_isPlugin()) return;
     if (globalState.pluginConfig.hasFlag(Plugins.AFTER_POSITION_MODIFY_FLAG)) {
       IAlgebraPlugin(plugin).afterModifyPosition(msg.sender, owner, bTick, tTick, deltaL, amount0, amount1, data).shouldReturn(
         IAlgebraPlugin.afterModifyPosition.selector
@@ -195,13 +223,19 @@ contract AlgebraPool is AlgebraPoolBase, TickStructure, ReentrancyGuard, Positio
 
         if (amount0 > 0) _transfer(token0, recipient, amount0);
         if (amount1 > 0) _transfer(token1, recipient, amount1);
-        _changeReserves(-int256(uint256(amount0)), -int256(uint256(amount1)), 0, 0);
+        _changeReserves(-int256(uint256(amount0)), -int256(uint256(amount1)), 0, 0, 0, 0);
       }
       emit Collect(msg.sender, recipient, bottomTick, topTick, amount0, amount1);
     }
     _unlock();
   }
 
+  struct SwapEventParams {
+    uint160 currentPrice;
+    int24 currentTick;
+    uint128 currentLiquidity;
+  }
+
   /// @inheritdoc IAlgebraPoolActions
   function swap(
     address recipient,
@@ -210,34 +244,38 @@ contract AlgebraPool is AlgebraPoolBase, TickStructure, ReentrancyGuard, Positio
     uint160 limitSqrtPrice,
     bytes calldata data
   ) external override returns (int256 amount0, int256 amount1) {
-    _beforeSwap(recipient, zeroToOne, amountRequired, limitSqrtPrice, false, data);
+    (uint24 overrideFee, uint24 pluginFee) = _beforeSwap(recipient, zeroToOne, amountRequired, limitSqrtPrice, false, data);
     _lock();
 
     {
       // scope to prevent "stack too deep"
+      SwapEventParams memory eventParams;
+      FeesAmount memory fees;
+      (amount0, amount1, eventParams.currentPrice, eventParams.currentTick, eventParams.currentLiquidity, fees) = _calculateSwap(
+        overrideFee,
+        pluginFee,
+        zeroToOne,
+        amountRequired,
+        limitSqrtPrice
+      );
       (uint256 balance0Before, uint256 balance1Before) = _updateReserves();
-      uint160 currentPrice;
-      int24 currentTick;
-      uint128 currentLiquidity;
-      uint256 communityFee;
-      (amount0, amount1, currentPrice, currentTick, currentLiquidity, communityFee) = _calculateSwap(zeroToOne, amountRequired, limitSqrtPrice);
       if (zeroToOne) {
         unchecked {
           if (amount1 < 0) _transfer(token1, recipient, uint256(-amount1)); // amount1 cannot be > 0
         }
         _swapCallback(amount0, amount1, data); // callback to get tokens from the msg.sender
         if (balance0Before + uint256(amount0) > _balanceToken0()) revert insufficientInputAmount();
-        _changeReserves(amount0, amount1, communityFee, 0); // reflect reserve change and pay communityFee
+        _changeReserves(amount0, amount1, fees.communityFeeAmount, 0, fees.pluginFeeAmount, 0); // reflect reserve change and pay communityFee
       } else {
         unchecked {
           if (amount0 < 0) _transfer(token0, recipient, uint256(-amount0)); // amount0 cannot be > 0
         }
         _swapCallback(amount0, amount1, data); // callback to get tokens from the msg.sender
         if (balance1Before + uint256(amount1) > _balanceToken1()) revert insufficientInputAmount();
-        _changeReserves(amount0, amount1, 0, communityFee); // reflect reserve change and pay communityFee
+        _changeReserves(amount0, amount1, 0, fees.communityFeeAmount, 0, fees.pluginFeeAmount); // reflect reserve change and pay communityFee
       }
 
-      _emitSwapEvent(recipient, amount0, amount1, currentPrice, currentLiquidity, currentTick);
+      _emitSwapEvent(recipient, amount0, amount1, eventParams.currentPrice, eventParams.currentLiquidity, eventParams.currentTick);
     }
 
     _unlock();
@@ -266,46 +304,50 @@ contract AlgebraPool is AlgebraPoolBase, TickStructure, ReentrancyGuard, Positio
         _swapCallback(amountToSell, 0, data); // callback to get tokens from the msg.sender
         uint256 balanceAfter = _balanceToken0();
         amountReceived = (balanceAfter - balanceBefore).toInt256();
-        _changeReserves(amountReceived, 0, 0, 0);
+        _changeReserves(amountReceived, 0, 0, 0, 0, 0);
       } else {
         uint256 balanceBefore = _balanceToken1();
         _swapCallback(0, amountToSell, data); // callback to get tokens from the msg.sender
         uint256 balanceAfter = _balanceToken1();
         amountReceived = (balanceAfter - balanceBefore).toInt256();
-        _changeReserves(0, amountReceived, 0, 0);
+        _changeReserves(0, amountReceived, 0, 0, 0, 0);
       }
       if (amountReceived != amountToSell) amountToSell = amountReceived;
     }
     if (amountToSell == 0) revert insufficientInputAmount();
 
     _unlock();
-    _beforeSwap(recipient, zeroToOne, amountToSell, limitSqrtPrice, true, data);
+    (uint24 overrideFee, uint24 pluginFee) = _beforeSwap(recipient, zeroToOne, amountToSell, limitSqrtPrice, true, data);
     _lock();
 
     _updateReserves();
 
-    uint160 currentPrice;
-    int24 currentTick;
-    uint128 currentLiquidity;
-    uint256 communityFee;
-    (amount0, amount1, currentPrice, currentTick, currentLiquidity, communityFee) = _calculateSwap(zeroToOne, amountToSell, limitSqrtPrice);
+    SwapEventParams memory eventParams;
+    FeesAmount memory fees;
+    (amount0, amount1, eventParams.currentPrice, eventParams.currentTick, eventParams.currentLiquidity, fees) = _calculateSwap(
+      overrideFee,
+      pluginFee,
+      zeroToOne,
+      amountToSell,
+      limitSqrtPrice
+    );
 
     unchecked {
       // transfer to the recipient
       if (zeroToOne) {
         if (amount1 < 0) _transfer(token1, recipient, uint256(-amount1)); // amount1 cannot be > 0
         uint256 leftover = uint256(amountToSell - amount0); // return the leftovers
         if (leftover != 0) _transfer(token0, leftoversRecipient, leftover);
-        _changeReserves(-leftover.toInt256(), amount1, communityFee, 0); // reflect reserve change and pay communityFee
+        _changeReserves(-leftover.toInt256(), amount1, fees.communityFeeAmount, 0, fees.pluginFeeAmount, 0); // reflect reserve change and pay communityFee
       } else {
         if (amount0 < 0) _transfer(token0, recipient, uint256(-amount0)); // amount0 cannot be > 0
         uint256 leftover = uint256(amountToSell - amount1); // return the leftovers
         if (leftover != 0) _transfer(token1, leftoversRecipient, leftover);
-        _changeReserves(amount0, -leftover.toInt256(), 0, communityFee); // reflect reserve change and pay communityFee
+        _changeReserves(amount0, -leftover.toInt256(), 0, fees.communityFeeAmount, 0, fees.pluginFeeAmount); // reflect reserve change and pay communityFee
       }
     }
 
-    _emitSwapEvent(recipient, amount0, amount1, currentPrice, currentLiquidity, currentTick);
+    _emitSwapEvent(recipient, amount0, amount1, eventParams.currentPrice, eventParams.currentLiquidity, eventParams.currentTick);
 
     _unlock();
     _afterSwap(recipient, zeroToOne, amountToSell, limitSqrtPrice, amount0, amount1, data);
@@ -316,16 +358,26 @@ contract AlgebraPool is AlgebraPoolBase, TickStructure, ReentrancyGuard, Positio
     emit Swap(msg.sender, recipient, amount0, amount1, newPrice, newLiquidity, newTick);
   }
 
-  function _beforeSwap(address recipient, bool zto, int256 amount, uint160 limitPrice, bool payInAdvance, bytes calldata data) internal {
+  function _beforeSwap(
+    address recipient,
+    bool zto,
+    int256 amount,
+    uint160 limitPrice,
+    bool payInAdvance,
+    bytes calldata data
+  ) internal returns (uint24 overrideFee, uint24 pluginFee) {
     if (globalState.pluginConfig.hasFlag(Plugins.BEFORE_SWAP_FLAG)) {
-      IAlgebraPlugin(plugin).beforeSwap(msg.sender, recipient, zto, amount, limitPrice, payInAdvance, data).shouldReturn(
-        IAlgebraPlugin.beforeSwap.selector
-      );
+      if (_isPlugin()) return (0, 0);
+      bytes4 selector;
+      (selector, overrideFee, pluginFee) = IAlgebraPlugin(plugin).beforeSwap(msg.sender, recipient, zto, amount, limitPrice, payInAdvance, data);
+      if (overrideFee >= 1e6 || pluginFee > overrideFee) revert incorrectPluginFee();
+      selector.shouldReturn(IAlgebraPlugin.beforeSwap.selector);
     }
   }
 
   function _afterSwap(address recipient, bool zto, int256 amount, uint160 limitPrice, int256 amount0, int256 amount1, bytes calldata data) internal {
     if (globalState.pluginConfig.hasFlag(Plugins.AFTER_SWAP_FLAG)) {
+      if (_isPlugin()) return;
       IAlgebraPlugin(plugin).afterSwap(msg.sender, recipient, zto, amount, limitPrice, amount0, amount1, data).shouldReturn(
         IAlgebraPlugin.afterSwap.selector
       );
@@ -373,7 +425,7 @@ contract AlgebraPool is AlgebraPoolBase, TickStructure, ReentrancyGuard, Positio
         uint256 communityFee1;
         if (paid1 > 0) communityFee1 = FullMath.mulDiv(paid1, _communityFee, Constants.COMMUNITY_FEE_DENOMINATOR);
 
-        _changeReserves(int256(communityFee0), int256(communityFee1), communityFee0, communityFee1);
+        _changeReserves(int256(communityFee0), int256(communityFee1), communityFee0, communityFee1, 0, 0);
       }
       emit Flash(msg.sender, recipient, amount0, amount1, paid0, paid1);
     }

src/core/contracts/base/AlgebraPoolBase.sol

@@ -63,11 +63,14 @@ abstract contract AlgebraPoolBase is IAlgebraPool, Timestamp {
   /// @inheritdoc IAlgebraPoolState
   mapping(int24 => TickManagement.Tick) public override ticks;
 
-  /// @inheritdoc IAlgebraPoolState
-  uint32 public override communityFeeLastTimestamp;
   /// @dev The amounts of token0 and token1 that will be sent to the vault
   uint104 internal communityFeePending0;
   uint104 internal communityFeePending1;
+  /// @inheritdoc IAlgebraPoolState
+  uint32 public override lastFeeTransferTimestamp;
+
+  uint104 internal pluginFeePending0;
+  uint104 internal pluginFeePending1;
 
   /// @inheritdoc IAlgebraPoolState
   address public override plugin;
@@ -134,6 +137,10 @@ abstract contract AlgebraPoolBase is IAlgebraPool, Timestamp {
     return (communityFeePending0, communityFeePending1);
   }
 
+  function getPluginFeePending() external view override returns (uint128, uint128) {
+    return (pluginFeePending0, pluginFeePending1);
+  }
+
   /// @inheritdoc IAlgebraPoolState
   function fee() external view override returns (uint16 currentFee) {
     currentFee = globalState.lastFee;