diff --git a/.forge-snapshots/initialize.snap b/.forge-snapshots/initialize.snap index 1d25aeb8b..236da0ded 100644 --- a/.forge-snapshots/initialize.snap +++ b/.forge-snapshots/initialize.snap @@ -1 +1 @@ -59966 \ No newline at end of file +60021 \ No newline at end of file diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap index 3fd5dc3c4..2c7a2c1d9 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -24130 \ No newline at end of file +24119 \ No newline at end of file diff --git a/foundry.toml b/foundry.toml index eeb148d1b..1e633aaed 100644 --- a/foundry.toml +++ b/foundry.toml @@ -5,6 +5,7 @@ ffi = true fs_permissions = [{ access = "read-write", path = ".forge-snapshots/"}, { access = "read", path = "./out"}, {access = "read", path = "./test/bin"}] solc = "0.8.26" evm_version = "cancun" +gas_limit = "300000000" [profile.default.fuzz] runs = 1000 diff --git a/src/PoolManager.sol b/src/PoolManager.sol index 4eb5b232e..5cfbecbe4 100644 --- a/src/PoolManager.sol +++ b/src/PoolManager.sol @@ -92,8 +92,6 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim mapping(PoolId id => Pool.State) internal _pools; - constructor(uint256 controllerGasLimit) ProtocolFees(controllerGasLimit) {} - /// @notice This will revert if the contract is locked modifier onlyWhenUnlocked() { if (!Lock.isUnlocked()) ManagerLocked.selector.revertWith(); diff --git a/src/ProtocolFees.sol b/src/ProtocolFees.sol index 29371a31f..f89566872 100644 --- a/src/ProtocolFees.sol +++ b/src/ProtocolFees.sol @@ -6,6 +6,7 @@ import {IProtocolFeeController} from "./interfaces/IProtocolFeeController.sol"; import {IProtocolFees} from "./interfaces/IProtocolFees.sol"; import {PoolKey} from "./types/PoolKey.sol"; import {ProtocolFeeLibrary} from "./libraries/ProtocolFeeLibrary.sol"; +import {BipsLibrary} from "./libraries/BipsLibrary.sol"; import {Owned} from "solmate/src/auth/Owned.sol"; import {PoolId} from "./types/PoolId.sol"; import {Pool} from "./libraries/Pool.sol"; @@ -16,6 +17,7 @@ abstract contract ProtocolFees is IProtocolFees, Owned { using ProtocolFeeLibrary for uint24; using Pool for Pool.State; using CustomRevert for bytes4; + using BipsLibrary for uint256; /// @inheritdoc IProtocolFees mapping(Currency currency => uint256 amount) public protocolFeesAccrued; @@ -23,11 +25,11 @@ abstract contract ProtocolFees is IProtocolFees, Owned { /// @inheritdoc IProtocolFees IProtocolFeeController public protocolFeeController; - uint256 private immutable controllerGasLimit; + // a percentage of the block.gaslimit denoted in basis points, used as the gas limit for fee controller calls + // 100 bps is 1%, at 30M gas, the limit is 300K + uint256 private constant BLOCK_LIMIT_BPS = 100; - constructor(uint256 _controllerGasLimit) Owned(msg.sender) { - controllerGasLimit = _controllerGasLimit; - } + constructor() Owned(msg.sender) {} /// @inheritdoc IProtocolFees function setProtocolFeeController(IProtocolFeeController controller) external onlyOwner { @@ -70,12 +72,13 @@ abstract contract ProtocolFees is IProtocolFees, Owned { /// the success of this function is NOT checked on initialize and if the call fails, the protocol fees are set to 0. function _fetchProtocolFee(PoolKey memory key) internal returns (uint24 protocolFee) { if (address(protocolFeeController) != address(0)) { + uint256 controllerGasLimit = block.gaslimit.calculatePortion(BLOCK_LIMIT_BPS); + // note that EIP-150 mandates that calls requesting more than 63/64ths of remaining gas // will be allotted no more than this amount, so controllerGasLimit must be set with this // in mind. if (gasleft() < controllerGasLimit) ProtocolFeeCannotBeFetched.selector.revertWith(); - uint256 gasLimit = controllerGasLimit; address toAddress = address(protocolFeeController); bytes memory data = abi.encodeCall(IProtocolFeeController.protocolFeeForPool, (key)); @@ -84,7 +87,7 @@ abstract contract ProtocolFees is IProtocolFees, Owned { uint256 returnData; assembly ("memory-safe") { // only load the first 32 bytes of the return data to prevent gas griefing - success := call(gasLimit, toAddress, 0, add(data, 0x20), mload(data), 0, 32) + success := call(controllerGasLimit, toAddress, 0, add(data, 0x20), mload(data), 0, 32) // if success is false this wont actually be returned, instead 0 will be returned returnData := mload(0) diff --git a/src/libraries/BipsLibrary.sol b/src/libraries/BipsLibrary.sol new file mode 100644 index 000000000..219232e25 --- /dev/null +++ b/src/libraries/BipsLibrary.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +pragma solidity ^0.8.0; + +/// @title For calculating a percentage of an amount, using bips +library BipsLibrary { + uint256 internal constant BPS_DENOMINATOR = 10_000; + + /// @notice emitted when an invalid percentage is provided + error InvalidBips(); + + /// @param amount The total amount to calculate a percentage of + /// @param bips The percentage to calculate, in bips + function calculatePortion(uint256 amount, uint256 bips) internal pure returns (uint256) { + if (bips > BPS_DENOMINATOR) revert InvalidBips(); + return (amount * bips) / BPS_DENOMINATOR; + } +} diff --git a/src/test/ProtocolFeesImplementation.sol b/src/test/ProtocolFeesImplementation.sol index ee31b1f84..30008e7dc 100644 --- a/src/test/ProtocolFeesImplementation.sol +++ b/src/test/ProtocolFeesImplementation.sol @@ -12,8 +12,6 @@ contract ProtocolFeesImplementation is ProtocolFees { mapping(PoolId id => Pool.State) internal _pools; bool internal isUnlocked; - constructor(uint256 _controllerGasLimit) ProtocolFees(_controllerGasLimit) {} - // Used to set the price of a pool to pretend that the pool has been initialized in order to successfully set a protocol fee function setPrice(PoolKey memory key, uint160 sqrtPriceX96) public { Pool.State storage pool = _getPool(key.toId()); diff --git a/src/test/ProxyPoolManager.sol b/src/test/ProxyPoolManager.sol index bfcfa35e6..f8f177be2 100644 --- a/src/test/ProxyPoolManager.sol +++ b/src/test/ProxyPoolManager.sol @@ -44,7 +44,7 @@ contract ProxyPoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909 address internal immutable _delegateManager; - constructor(address delegateManager, uint256 controllerGasLimit) ProtocolFees(controllerGasLimit) { + constructor(address delegateManager) { _delegateManager = delegateManager; } diff --git a/test/NoDelegateCall.t.sol b/test/NoDelegateCall.t.sol index 61173ecb6..4aa1f7925 100644 --- a/test/NoDelegateCall.t.sol +++ b/test/NoDelegateCall.t.sol @@ -14,8 +14,8 @@ import {Deployers} from "./utils/Deployers.sol"; contract TestDelegateCall is Test, Deployers, GasSnapshot { // override to use ProxyPoolManager function deployFreshManager() internal virtual override { - IPoolManager delegateManager = new PoolManager(500000); - manager = new ProxyPoolManager(address(delegateManager), 500000); + IPoolManager delegateManager = new PoolManager(); + manager = new ProxyPoolManager(address(delegateManager)); } NoDelegateCallTest noDelegateCallTest; diff --git a/test/ProtocolFeesImplementation.t.sol b/test/ProtocolFeesImplementation.t.sol index fc78ad785..716d8b1bf 100644 --- a/test/ProtocolFeesImplementation.t.sol +++ b/test/ProtocolFeesImplementation.t.sol @@ -33,7 +33,7 @@ contract ProtocolFeesTest is Test, GasSnapshot, Deployers { ProtocolFeesImplementation protocolFees; function setUp() public { - protocolFees = new ProtocolFeesImplementation(5000); + protocolFees = new ProtocolFeesImplementation(); feeController = new ProtocolFeeControllerTest(); (currency0, currency1) = deployAndMint2Currencies(); MockERC20(Currency.unwrap(currency0)).transfer(address(protocolFees), 2 ** 255); diff --git a/test/libraries/BipsLibrary.t.sol b/test/libraries/BipsLibrary.t.sol new file mode 100644 index 000000000..02cc67d71 --- /dev/null +++ b/test/libraries/BipsLibrary.t.sol @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import "forge-std/Test.sol"; +import {BipsLibrary} from "../../src/libraries/BipsLibrary.sol"; + +contract BipsLibraryTest is Test { + using BipsLibrary for uint256; + + // The block gas limit set in foundry config is 300_000_000 (300M) for testing purposes + uint256 BLOCK_GAS_LIMIT; + + function setUp() public { + BLOCK_GAS_LIMIT = block.gaslimit; + } + + function test_fuzz_calculatePortion(uint256 amount, uint256 bips) public { + amount = bound(amount, 0, uint256(type(uint128).max)); + if (bips > BipsLibrary.BPS_DENOMINATOR) { + vm.expectRevert(BipsLibrary.InvalidBips.selector); + amount.calculatePortion(bips); + } else { + assertEq(amount.calculatePortion(bips), amount * bips / BipsLibrary.BPS_DENOMINATOR); + } + } + + function test_fuzz_gasLimit(uint256 bips) public { + if (bips > BipsLibrary.BPS_DENOMINATOR) { + vm.expectRevert(BipsLibrary.InvalidBips.selector); + block.gaslimit.calculatePortion(bips); + } else { + assertEq(block.gaslimit.calculatePortion(bips), BLOCK_GAS_LIMIT * bips / BipsLibrary.BPS_DENOMINATOR); + } + } + + function test_gasLimit_100_percent() public view { + assertEq(block.gaslimit, block.gaslimit.calculatePortion(10_000)); + } + + function test_gasLimit_1_percent() public view { + // 100 bps = 1% + // 1% of 30M is 300K + assertEq(BLOCK_GAS_LIMIT / 100, block.gaslimit.calculatePortion(100)); + } + + function test_gasLimit_1BP() public view { + // 1bp is 0.01% + // 0.01% of 30M is 300 + assertEq(BLOCK_GAS_LIMIT / 10000, block.gaslimit.calculatePortion(1)); + } +} diff --git a/test/utils/Deployers.sol b/test/utils/Deployers.sol index 449017625..2d7b6f410 100644 --- a/test/utils/Deployers.sol +++ b/test/utils/Deployers.sol @@ -85,7 +85,7 @@ contract Deployers { uint160 clearAllHookPermissionsMask = ~uint160(0) << (hookPermissionCount); function deployFreshManager() internal virtual { - manager = new PoolManager(500000); + manager = new PoolManager(); } function deployFreshManagerAndRouters() internal {