BaseFeeCalculator.sol

[HIGH] trackSwap calls are not limited to PositionManager

In the StaticFeeCalculator anyone is able to call the trackSwap function. From previous code reviews this was never an issue, however as the there is now an internal rerouting based on fair launch status, it means that anyone can proxy call the TrustedSignerFeeCalculator.trackSwap function. This was previously locked down via a role grant, but StaticFeeCalculator would need to have this role which would in turn allow any proxy routed calls to hit the TrustedSignerFeeCalculator function.

To prevent this, we would recommend moving the AccessControl inheritence to the BaseFeeCalculator and including this validation in the deployment script for each calculator individually.

[HIGH] Unchecked External Call Return Values

If a valid manager is called that attempts to determine the swap fee, but reverts inside of the call, then the transaction will complete but will fallback on the default Flaunch swap fee (1%).

This could be a protocol decision, but it would be our recommendation that if the external manager reverts whilst calculating a swap fee, then the swap should be reverted. This can be achieved by using call or staticcall, rather than a try/catch:

function determineSwapFee(
  PoolKey memory _poolKey,
  IPoolManager.SwapParams memory _params,
  uint24 _baseFee
) public view returns (uint24 swapFee_) {
  // Check if there's a manager override
  address manager = _getApprovedSwapFeeManager(_poolKey);
  if (manager != address(0)) {
    // Check if the manager has the determineSwapFee function
    (bool success, bytes memory data) = manager.staticcall(
      abi.encodeWithSelector(
        ICustomFeeManager.determineSwapFee.selector,
        _poolKey,
        _params,
        _baseFee
      )
    );
    
    // Function exists and returned a value, decode and return it
    if (success && data.length >= 32) {
      return abi.decode(data, (uint24));
    }
  }

  // Call the default implementation
  return _determineSwapFee(_poolKey, _params, _baseFee);
}

This solution would need to be implemented for both determineSwapFee and trackSwap.

[MED] Potential Reentrancy in Manager Override Logic

External calls to custom fee managers in determineSwapFee and trackSwap do not have any reentrancy protection. We would strongly recommend adding a nonReentrant modifier to each of these functions to prevent malicious manager code execution.

[INFO] setFlaunchParams should not have virtual or override modifiers

From our understanding of the contract extensibility, the setFlaunchParams function should neither be overridden (it calls the virtual internal function _setFlaunchParams), nor does it override from anything.

[INFO] Pass fair launch status for external manager simplification

To promote a simplified building experience for third parties, we would recommend introducing a bool _fairLaunch parameter to both the external ICustomFeeManager and internal BaseFeeCalculator function calls, allowing them to digest fair launch state without requiring additional development and calculation. This would infer a small gas cost for all function calls, rather than just some. But the ease in building will help to promote a simplified building experience.