diff --git a/.forge-snapshots/add liquidity to already existing position with salt.snap b/.forge-snapshots/add liquidity to already existing position with salt.snap index 63187d056..3a7de6606 100644 --- a/.forge-snapshots/add liquidity to already existing position with salt.snap +++ b/.forge-snapshots/add liquidity to already existing position with salt.snap @@ -1 +1 @@ -144401 \ No newline at end of file +144425 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap index 6c4925203..d6d5405e1 100644 --- a/.forge-snapshots/addLiquidity CA fee.snap +++ b/.forge-snapshots/addLiquidity CA fee.snap @@ -1 +1 @@ -170697 \ No newline at end of file +170721 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap index cf57e9c87..cce113cd6 100644 --- a/.forge-snapshots/addLiquidity with empty hook.snap +++ b/.forge-snapshots/addLiquidity with empty hook.snap @@ -1 +1 @@ -274002 \ No newline at end of file +274026 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap index 51cc148cf..7ce5c036b 100644 --- a/.forge-snapshots/addLiquidity with native token.snap +++ b/.forge-snapshots/addLiquidity with native token.snap @@ -1 +1 @@ -135010 \ No newline at end of file +135022 \ No newline at end of file diff --git a/.forge-snapshots/create new liquidity to a position with salt.snap b/.forge-snapshots/create new liquidity to a position with salt.snap index fb9f94df6..9f41b5653 100644 --- a/.forge-snapshots/create new liquidity to a position with salt.snap +++ b/.forge-snapshots/create new liquidity to a position with salt.snap @@ -1 +1 @@ -292593 \ No newline at end of file +292617 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 1 token.snap b/.forge-snapshots/donate gas with 1 token.snap index 76ef304a0..1957407f7 100644 --- a/.forge-snapshots/donate gas with 1 token.snap +++ b/.forge-snapshots/donate gas with 1 token.snap @@ -1 +1 @@ -106202 \ No newline at end of file +106226 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 2 tokens.snap b/.forge-snapshots/donate gas with 2 tokens.snap index 1c2000f84..4f56f3bf1 100644 --- a/.forge-snapshots/donate gas with 2 tokens.snap +++ b/.forge-snapshots/donate gas with 2 tokens.snap @@ -1 +1 @@ -145504 \ No newline at end of file +145528 \ No newline at end of file diff --git a/.forge-snapshots/erc20 collect protocol fees.snap b/.forge-snapshots/erc20 collect protocol fees.snap index b9a086c0f..fdc5dcc0f 100644 --- a/.forge-snapshots/erc20 collect protocol fees.snap +++ b/.forge-snapshots/erc20 collect protocol fees.snap @@ -1 +1 @@ -57326 \ No newline at end of file +57338 \ No newline at end of file diff --git a/.forge-snapshots/initialize.snap b/.forge-snapshots/initialize.snap index 57a866726..450c7974e 100644 --- a/.forge-snapshots/initialize.snap +++ b/.forge-snapshots/initialize.snap @@ -1 +1 @@ -59536 \ No newline at end of file +51536 \ No newline at end of file diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap index 720e42d76..53b94643d 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -23884 \ No newline at end of file +23504 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity CA fee.snap b/.forge-snapshots/removeLiquidity CA fee.snap index 5b9da2ead..3ea79996d 100644 --- a/.forge-snapshots/removeLiquidity CA fee.snap +++ b/.forge-snapshots/removeLiquidity CA fee.snap @@ -1 +1 @@ -141195 \ No newline at end of file +141219 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap index 12528ddad..07ce00e06 100644 --- a/.forge-snapshots/removeLiquidity with empty hook.snap +++ b/.forge-snapshots/removeLiquidity with empty hook.snap @@ -1 +1 @@ -130597 \ No newline at end of file +130621 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap index 8e41d5a6a..35f7a9f26 100644 --- a/.forge-snapshots/removeLiquidity with native token.snap +++ b/.forge-snapshots/removeLiquidity with native token.snap @@ -1 +1 @@ -112525 \ No newline at end of file +112535 \ No newline at end of file diff --git a/.forge-snapshots/simple addLiquidity second addition same range.snap b/.forge-snapshots/simple addLiquidity second addition same range.snap index e320b098c..3788a673e 100644 --- a/.forge-snapshots/simple addLiquidity second addition same range.snap +++ b/.forge-snapshots/simple addLiquidity second addition same range.snap @@ -1 +1 @@ -98719 \ No newline at end of file +98743 \ No newline at end of file diff --git a/.forge-snapshots/simple addLiquidity.snap b/.forge-snapshots/simple addLiquidity.snap index 89697d003..adaea8624 100644 --- a/.forge-snapshots/simple addLiquidity.snap +++ b/.forge-snapshots/simple addLiquidity.snap @@ -1 +1 @@ -161264 \ No newline at end of file +161288 \ No newline at end of file diff --git a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap index b3bbea3f2..94c9272f7 100644 --- a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap +++ b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap @@ -1 +1 @@ -92971 \ No newline at end of file +92995 \ No newline at end of file diff --git a/.forge-snapshots/simple removeLiquidity.snap b/.forge-snapshots/simple removeLiquidity.snap index d8fffba4e..f12c87689 100644 --- a/.forge-snapshots/simple removeLiquidity.snap +++ b/.forge-snapshots/simple removeLiquidity.snap @@ -1 +1 @@ -85084 \ No newline at end of file +85108 \ No newline at end of file diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap index f7a10cf66..4bf8e4447 100644 --- a/.forge-snapshots/simple swap with native.snap +++ b/.forge-snapshots/simple swap with native.snap @@ -1 +1 @@ -108503 \ No newline at end of file +108515 \ No newline at end of file diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap index 7d7fb73b7..2da45ab6c 100644 --- a/.forge-snapshots/simple swap.snap +++ b/.forge-snapshots/simple swap.snap @@ -1 +1 @@ -123204 \ No newline at end of file +123228 \ No newline at end of file diff --git a/.forge-snapshots/swap CA custom curve + swap noop.snap b/.forge-snapshots/swap CA custom curve + swap noop.snap index 481863692..d55922597 100644 --- a/.forge-snapshots/swap CA custom curve + swap noop.snap +++ b/.forge-snapshots/swap CA custom curve + swap noop.snap @@ -1 +1 @@ -124391 \ No newline at end of file +124415 \ No newline at end of file diff --git a/.forge-snapshots/swap CA fee on unspecified.snap b/.forge-snapshots/swap CA fee on unspecified.snap index 96eb57794..9a4dee558 100644 --- a/.forge-snapshots/swap CA fee on unspecified.snap +++ b/.forge-snapshots/swap CA fee on unspecified.snap @@ -1 +1 @@ -154630 \ No newline at end of file +154654 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap index f40f02cd3..24cd0f5c3 100644 --- a/.forge-snapshots/swap against liquidity with native token.snap +++ b/.forge-snapshots/swap against liquidity with native token.snap @@ -1 +1 @@ -105625 \ No newline at end of file +105637 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap index ba2bafde0..8b00aa52e 100644 --- a/.forge-snapshots/swap against liquidity.snap +++ b/.forge-snapshots/swap against liquidity.snap @@ -1 +1 @@ -116574 \ No newline at end of file +116598 \ No newline at end of file diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap index c9fc24b91..8cfb479ef 100644 --- a/.forge-snapshots/swap burn 6909 for input.snap +++ b/.forge-snapshots/swap burn 6909 for input.snap @@ -1 +1 @@ -129257 \ No newline at end of file +129281 \ No newline at end of file diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap index 44d846156..4b0be0884 100644 --- a/.forge-snapshots/swap burn native 6909 for input.snap +++ b/.forge-snapshots/swap burn native 6909 for input.snap @@ -1 +1 @@ -118713 \ No newline at end of file +118725 \ No newline at end of file diff --git a/.forge-snapshots/swap mint native output as 6909.snap b/.forge-snapshots/swap mint native output as 6909.snap index c876ac085..276cb3353 100644 --- a/.forge-snapshots/swap mint native output as 6909.snap +++ b/.forge-snapshots/swap mint native output as 6909.snap @@ -1 +1 @@ -139616 \ No newline at end of file +139628 \ No newline at end of file diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap index 267543ae1..11cee36c8 100644 --- a/.forge-snapshots/swap mint output as 6909.snap +++ b/.forge-snapshots/swap mint output as 6909.snap @@ -1 +1 @@ -155045 \ No newline at end of file +155069 \ No newline at end of file diff --git a/.forge-snapshots/swap skips hook call if hook is caller.snap b/.forge-snapshots/swap skips hook call if hook is caller.snap index af49d2996..93dbce2bc 100644 --- a/.forge-snapshots/swap skips hook call if hook is caller.snap +++ b/.forge-snapshots/swap skips hook call if hook is caller.snap @@ -1 +1 @@ -206177 \ No newline at end of file +206165 \ No newline at end of file diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap index f7b98701c..552bf8fb9 100644 --- a/.forge-snapshots/swap with hooks.snap +++ b/.forge-snapshots/swap with hooks.snap @@ -1 +1 @@ -132202 \ No newline at end of file +132226 \ No newline at end of file diff --git a/.forge-snapshots/swap with return dynamic fee.snap b/.forge-snapshots/swap with return dynamic fee.snap index 4b5df658a..84f5b56a2 100644 --- a/.forge-snapshots/swap with return dynamic fee.snap +++ b/.forge-snapshots/swap with return dynamic fee.snap @@ -1 +1 @@ -145554 \ No newline at end of file +145542 \ No newline at end of file diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 000000000..f03633465 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1 @@ +* @uniswap/protocols diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 0bcf3c9de..e5524a5f7 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -32,7 +32,7 @@ jobs: run: | { echo 'COVERAGE<> "$GITHUB_OUTPUT" env: @@ -40,9 +40,12 @@ jobs: - name: Check coverage is updated uses: actions/github-script@v5 + env: + OUTPUT: ${{ steps.coverage.outputs.COVERAGE }} with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | + const { OUTPUT } = process.env; const fs = require('fs'); const file = "coverage.txt" if(!fs.existsSync(file)) { @@ -50,7 +53,7 @@ jobs: return } const currentCoverage = fs.readFileSync(file, "utf8").trim(); - const newCoverage = (`${{ steps.coverage.outputs.COVERAGE }}`).trim(); + const newCoverage = (`${ OUTPUT }`).trim(); if (newCoverage != currentCoverage) { core.setFailed(`Code coverage not updated. Run : forge coverage | grep '^|' | grep -v 'test/' > coverage.txt`); } @@ -58,9 +61,12 @@ jobs: - name: Comment on PR id: comment uses: actions/github-script@v5 + env: + OUTPUT: ${{ steps.coverage.outputs.COVERAGE }} with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | + const { OUTPUT } = process.env; const {data: comments} = await github.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, @@ -69,8 +75,7 @@ jobs: const botComment = comments.find(comment => comment.user.id === 41898282) - const output = `${{ steps.coverage.outputs.COVERAGE }}`; - const commentBody = `Forge code coverage:\n${output}\n`; + const commentBody = `Forge code coverage:\n${ OUTPUT }\n`; if (botComment) { github.rest.issues.updateComment({ diff --git a/.github/workflows/tests-merge.yml b/.github/workflows/tests-merge.yml index 13da03c2b..e1b11646d 100644 --- a/.github/workflows/tests-merge.yml +++ b/.github/workflows/tests-merge.yml @@ -1,6 +1,7 @@ name: Tests on: + workflow_dispatch: push: branches: - main @@ -8,7 +9,8 @@ on: jobs: forge-tests: name: Forge Tests - runs-on: ubuntu-latest + runs-on: + group: forge-testing steps: - uses: actions/checkout@v3 diff --git a/src/PoolManager.sol b/src/PoolManager.sol index d8a735c1f..6b3a83b13 100644 --- a/src/PoolManager.sol +++ b/src/PoolManager.sol @@ -128,9 +128,8 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim key.hooks.beforeInitialize(key, sqrtPriceX96); PoolId id = key.toId(); - uint24 protocolFee = _fetchProtocolFee(key); - tick = _pools[id].initialize(sqrtPriceX96, protocolFee, lpFee); + tick = _pools[id].initialize(sqrtPriceX96, lpFee); key.hooks.afterInitialize(key, sqrtPriceX96, tick); @@ -174,7 +173,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim BalanceDelta hookDelta; (callerDelta, hookDelta) = key.hooks.afterModifyLiquidity(key, params, callerDelta, feesAccrued, hookData); - // if the hook doesnt have the flag to be able to return deltas, hookDelta will always be 0 + // if the hook doesn't have the flag to be able to return deltas, hookDelta will always be 0 if (hookDelta != BalanceDeltaLibrary.ZERO_DELTA) _accountPoolBalanceDelta(key, hookDelta, address(key.hooks)); _accountPoolBalanceDelta(key, callerDelta, msg.sender); @@ -217,7 +216,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim BalanceDelta hookDelta; (swapDelta, hookDelta) = key.hooks.afterSwap(key, params, swapDelta, hookData, beforeSwapDelta); - // if the hook doesnt have the flag to be able to return deltas, hookDelta will always be 0 + // if the hook doesn't have the flag to be able to return deltas, hookDelta will always be 0 if (hookDelta != BalanceDeltaLibrary.ZERO_DELTA) _accountPoolBalanceDelta(key, hookDelta, address(key.hooks)); _accountPoolBalanceDelta(key, swapDelta, msg.sender); diff --git a/src/ProtocolFees.sol b/src/ProtocolFees.sol index 1df3fcd23..6f00d5621 100644 --- a/src/ProtocolFees.sol +++ b/src/ProtocolFees.sol @@ -6,7 +6,6 @@ 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"; @@ -17,7 +16,6 @@ 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; @@ -25,10 +23,6 @@ abstract contract ProtocolFees is IProtocolFees, Owned { /// @inheritdoc IProtocolFees IProtocolFeeController public protocolFeeController; - // 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() Owned(msg.sender) {} /// @inheritdoc IProtocolFees @@ -65,42 +59,6 @@ abstract contract ProtocolFees is IProtocolFees, Owned { /// @dev this is overridden in PoolManager.sol to give access to the _pools mapping function _getPool(PoolId id) internal virtual returns (Pool.State storage); - /// @notice Fetch the protocol fees for a given pool - /// @dev the success of this function is false if the call fails or the returned fees are invalid - /// @dev to prevent an invalid protocol fee controller from blocking pools from being initialized - /// 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(); - - address toAddress = address(protocolFeeController); - - bytes memory data = abi.encodeCall(IProtocolFeeController.protocolFeeForPool, (key)); - - bool success; - uint256 returnData; - assembly ("memory-safe") { - // only load the first 32 bytes of the return data to prevent gas griefing - 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) - - // success if return data size is 32 bytes - success := and(success, eq(returndatasize(), 32)) - } - - // Ensure return data does not overflow a uint24 and that the underlying fees are within bounds. - protocolFee = success && (returnData == uint24(returnData)) && uint24(returnData).isValidProtocolFee() - ? uint24(returnData) - : 0; - } - } - function _updateProtocolFees(Currency currency, uint256 amount) internal { unchecked { protocolFeesAccrued[currency] += amount; diff --git a/src/interfaces/IProtocolFees.sol b/src/interfaces/IProtocolFees.sol index 7cf45d85f..4789266f7 100644 --- a/src/interfaces/IProtocolFees.sol +++ b/src/interfaces/IProtocolFees.sol @@ -8,8 +8,6 @@ import {PoolKey} from "../types/PoolKey.sol"; /// @notice Interface for all protocol-fee related functions in the pool manager interface IProtocolFees { - /// @notice Thrown when not enough gas is provided to look up the protocol fee - error ProtocolFeeCannotBeFetched(); /// @notice Thrown when protocol fee is set too high error ProtocolFeeTooLarge(uint24 fee); diff --git a/src/libraries/BipsLibrary.sol b/src/libraries/BipsLibrary.sol deleted file mode 100644 index 219232e25..000000000 --- a/src/libraries/BipsLibrary.sol +++ /dev/null @@ -1,17 +0,0 @@ -// 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/libraries/Hooks.sol b/src/libraries/Hooks.sol index e175d7bf5..2f02548f5 100644 --- a/src/libraries/Hooks.sol +++ b/src/libraries/Hooks.sol @@ -126,7 +126,7 @@ library Hooks { : (uint160(address(self)) & ALL_HOOK_MASK > 0 || fee.isDynamicFee()); } - /// @notice performs a hook call using the given calldata on the given hook that doesnt return a delta + /// @notice performs a hook call using the given calldata on the given hook that doesn't return a delta /// @return result The complete data returned by the hook function callHook(IHooks self, bytes memory data) internal returns (bytes memory result) { bool success; @@ -268,7 +268,7 @@ library Hooks { // any return in unspecified is passed to the afterSwap hook for handling int128 hookDeltaSpecified = hookReturn.getSpecifiedDelta(); - // Update the swap amount according to the hook's return, and check that the swap type doesnt change (exact input/output) + // Update the swap amount according to the hook's return, and check that the swap type doesn't change (exact input/output) if (hookDeltaSpecified != 0) { bool exactInput = amountToSwap < 0; amountToSwap += hookDeltaSpecified; diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol index 0f64f5e00..bdd9b3994 100644 --- a/src/libraries/Pool.sol +++ b/src/libraries/Pool.sol @@ -94,16 +94,13 @@ library Pool { if (tickUpper > TickMath.MAX_TICK) TickUpperOutOfBounds.selector.revertWith(tickUpper); } - function initialize(State storage self, uint160 sqrtPriceX96, uint24 protocolFee, uint24 lpFee) - internal - returns (int24 tick) - { + function initialize(State storage self, uint160 sqrtPriceX96, uint24 lpFee) internal returns (int24 tick) { if (self.slot0.sqrtPriceX96() != 0) PoolAlreadyInitialized.selector.revertWith(); tick = TickMath.getTickAtSqrtPrice(sqrtPriceX96); - self.slot0 = Slot0.wrap(bytes32(0)).setSqrtPriceX96(sqrtPriceX96).setTick(tick).setProtocolFee(protocolFee) - .setLpFee(lpFee); + // the initial protocolFee is 0 so doesn't need to be set + self.slot0 = Slot0.wrap(bytes32(0)).setSqrtPriceX96(sqrtPriceX96).setTick(tick).setLpFee(lpFee); } function setProtocolFee(State storage self, uint24 protocolFee) internal { @@ -404,7 +401,7 @@ library Pool { } // Shift tick if we reached the next price, and preemptively decrement for zeroForOne swaps to tickNext - 1. - // If the swap doesnt continue (if amountRemaining == 0 or sqrtPriceLimit is met), slot0.tick will be 1 less + // If the swap doesn't continue (if amountRemaining == 0 or sqrtPriceLimit is met), slot0.tick will be 1 less // than getTickAtSqrtPrice(slot0.sqrtPrice). This doesn't affect swaps, but donation calls should verify both // price and tick to reward the correct LPs. if (result.sqrtPriceX96 == step.sqrtPriceNextX96) { diff --git a/src/test/ProtocolFeeControllerTest.sol b/src/test/ProtocolFeeControllerTest.sol index dba8dd9cb..25fc0795d 100644 --- a/src/test/ProtocolFeeControllerTest.sol +++ b/src/test/ProtocolFeeControllerTest.sol @@ -17,42 +17,3 @@ contract ProtocolFeeControllerTest is IProtocolFeeController { protocolFee[id] = fee; } } - -/// @notice Reverts on call -contract RevertingProtocolFeeControllerTest is IProtocolFeeController { - function protocolFeeForPool(PoolKey memory /* key */ ) external pure returns (uint24) { - revert(); - } -} - -/// @notice Returns an out of bounds protocol fee -contract OutOfBoundsProtocolFeeControllerTest is IProtocolFeeController { - function protocolFeeForPool(PoolKey memory /* key */ ) external pure returns (uint24) { - // set both protocol fees to 1001, which is greater than MAX_PROTOCOL_FEE - return (1001 << 12) | 1001; - } -} - -/// @notice Return a value that overflows a uint24 -contract OverflowProtocolFeeControllerTest is IProtocolFeeController { - function protocolFeeForPool(PoolKey memory /* key */ ) external pure returns (uint24) { - assembly { - let ptr := mload(0x40) - mstore(ptr, 0xFFFFAAA001) - return(ptr, 0x20) - } - } -} - -/// @notice Returns data that is larger than a word -contract InvalidReturnSizeProtocolFeeControllerTest is IProtocolFeeController { - function protocolFeeForPool(PoolKey memory /* key */ ) external pure returns (uint24) { - address a = address(1); - assembly { - let ptr := mload(0x40) - mstore(ptr, a) - mstore(add(ptr, 0x20), a) - return(ptr, 0x40) - } - } -} diff --git a/src/test/ProtocolFeesImplementation.sol b/src/test/ProtocolFeesImplementation.sol index 30008e7dc..f485e26e1 100644 --- a/src/test/ProtocolFeesImplementation.sol +++ b/src/test/ProtocolFeesImplementation.sol @@ -30,10 +30,6 @@ contract ProtocolFeesImplementation is ProtocolFees { return isUnlocked; } - function fetchProtocolFee(PoolKey memory key) public returns (uint24) { - return ProtocolFees._fetchProtocolFee(key); - } - function updateProtocolFees(Currency currency, uint256 amount) public { ProtocolFees._updateProtocolFees(currency, amount); } diff --git a/src/test/ProxyPoolManager.sol b/src/test/ProxyPoolManager.sol index 1d9565c3f..a8a4415df 100644 --- a/src/test/ProxyPoolManager.sol +++ b/src/test/ProxyPoolManager.sol @@ -84,9 +84,8 @@ contract ProxyPoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909 key.hooks.beforeInitialize(key, sqrtPriceX96); PoolId id = key.toId(); - uint24 protocolFee = _fetchProtocolFee(key); - tick = _pools[id].initialize(sqrtPriceX96, protocolFee, lpFee); + tick = _pools[id].initialize(sqrtPriceX96, lpFee); key.hooks.afterInitialize(key, sqrtPriceX96, tick); diff --git a/test/PoolManagerInitialize.t.sol b/test/PoolManagerInitialize.t.sol index 39632f6a3..b4c9b43d0 100644 --- a/test/PoolManagerInitialize.t.sol +++ b/test/PoolManagerInitialize.t.sol @@ -226,24 +226,6 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { manager.initialize(uninitializedKey, sqrtPriceX96); } - function test_initialize_fetchFeeWhenController(uint24 protocolFee) public { - manager.setProtocolFeeController(feeController); - feeController.setProtocolFeeForPool(uninitializedKey.toId(), protocolFee); - - uint16 fee0 = protocolFee.getZeroForOneFee(); - uint16 fee1 = protocolFee.getOneForZeroFee(); - - manager.initialize(uninitializedKey, SQRT_PRICE_1_1); - - (uint160 slot0SqrtPriceX96,, uint24 slot0ProtocolFee,) = manager.getSlot0(uninitializedKey.toId()); - assertEq(slot0SqrtPriceX96, SQRT_PRICE_1_1); - if ((fee0 > 1000) || (fee1 > 1000)) { - assertEq(slot0ProtocolFee, 0); - } else { - assertEq(slot0ProtocolFee, protocolFee); - } - } - function test_initialize_revertsWhenPoolAlreadyInitialized(uint160 sqrtPriceX96) public { // Assumptions tested in Pool.t.sol sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_PRICE, TickMath.MAX_SQRT_PRICE - 1)); @@ -334,109 +316,6 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { manager.initialize(uninitializedKey, sqrtPriceX96); } - function test_initialize_succeedsWithOutOfBoundsFeeController(uint160 sqrtPriceX96) public { - // Assumptions tested in Pool.t.sol - sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_PRICE, TickMath.MAX_SQRT_PRICE - 1)); - - manager.setProtocolFeeController(outOfBoundsFeeController); - - int24 tick = TickMath.getTickAtSqrtPrice(sqrtPriceX96); - // expect initialize to succeed even though the controller reverts - vm.expectEmit(true, true, true, true); - emit Initialize( - uninitializedKey.toId(), - uninitializedKey.currency0, - uninitializedKey.currency1, - uninitializedKey.fee, - uninitializedKey.tickSpacing, - uninitializedKey.hooks, - sqrtPriceX96, - tick - ); - manager.initialize(uninitializedKey, sqrtPriceX96); - // protocol fees should default to 0 - (,, uint24 slot0ProtocolFee,) = manager.getSlot0(uninitializedKey.toId()); - assertEq(slot0ProtocolFee, 0); - } - - function test_initialize_succeedsWithRevertingFeeController(uint160 sqrtPriceX96) public { - // Assumptions tested in Pool.t.sol - sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_PRICE, TickMath.MAX_SQRT_PRICE - 1)); - - manager.setProtocolFeeController(revertingFeeController); - - int24 tick = TickMath.getTickAtSqrtPrice(sqrtPriceX96); - - // expect initialize to succeed even though the controller reverts - vm.expectEmit(true, true, true, true); - emit Initialize( - uninitializedKey.toId(), - uninitializedKey.currency0, - uninitializedKey.currency1, - uninitializedKey.fee, - uninitializedKey.tickSpacing, - uninitializedKey.hooks, - sqrtPriceX96, - tick - ); - manager.initialize(uninitializedKey, sqrtPriceX96); - // protocol fees should default to 0 - (,, uint24 slot0ProtocolFee,) = manager.getSlot0(uninitializedKey.toId()); - assertEq(slot0ProtocolFee, 0); - } - - function test_initialize_succeedsWithOverflowFeeController(uint160 sqrtPriceX96) public { - // Assumptions tested in Pool.t.sol - sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_PRICE, TickMath.MAX_SQRT_PRICE - 1)); - - manager.setProtocolFeeController(overflowFeeController); - - int24 tick = TickMath.getTickAtSqrtPrice(sqrtPriceX96); - - // expect initialize to succeed - vm.expectEmit(true, true, true, true); - emit Initialize( - uninitializedKey.toId(), - uninitializedKey.currency0, - uninitializedKey.currency1, - uninitializedKey.fee, - uninitializedKey.tickSpacing, - uninitializedKey.hooks, - sqrtPriceX96, - tick - ); - manager.initialize(uninitializedKey, sqrtPriceX96); - // protocol fees should default to 0 - (,, uint24 slot0ProtocolFee,) = manager.getSlot0(uninitializedKey.toId()); - assertEq(slot0ProtocolFee, 0); - } - - function test_initialize_succeedsWithWrongReturnSizeFeeController(uint160 sqrtPriceX96) public { - // Assumptions tested in Pool.t.sol - sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_PRICE, TickMath.MAX_SQRT_PRICE - 1)); - - manager.setProtocolFeeController(invalidReturnSizeFeeController); - - int24 tick = TickMath.getTickAtSqrtPrice(sqrtPriceX96); - - // expect initialize to succeed - vm.expectEmit(true, true, true, true); - emit Initialize( - uninitializedKey.toId(), - uninitializedKey.currency0, - uninitializedKey.currency1, - uninitializedKey.fee, - uninitializedKey.tickSpacing, - uninitializedKey.hooks, - sqrtPriceX96, - tick - ); - manager.initialize(uninitializedKey, sqrtPriceX96); - // protocol fees should default to 0 - (,, uint24 slot0ProtocolFee,) = manager.getSlot0(uninitializedKey.toId()); - assertEq(slot0ProtocolFee, 0); - } - function test_initialize_gas() public { manager.initialize(uninitializedKey, SQRT_PRICE_1_1); snapLastCall("initialize"); diff --git a/test/ProtocolFeesImplementation.t.sol b/test/ProtocolFeesImplementation.t.sol index 970b23112..f9692a133 100644 --- a/test/ProtocolFeesImplementation.t.sol +++ b/test/ProtocolFeesImplementation.t.sol @@ -14,13 +14,7 @@ import {Deployers} from "../test/utils/Deployers.sol"; import {PoolId} from "../src/types/PoolId.sol"; import {IHooks} from "../src/interfaces/IHooks.sol"; import {Constants} from "../test/utils/Constants.sol"; -import { - ProtocolFeeControllerTest, - OutOfBoundsProtocolFeeControllerTest, - RevertingProtocolFeeControllerTest, - OverflowProtocolFeeControllerTest, - InvalidReturnSizeProtocolFeeControllerTest -} from "../src/test/ProtocolFeeControllerTest.sol"; +import {ProtocolFeeControllerTest} from "../src/test/ProtocolFeeControllerTest.sol"; contract ProtocolFeesTest is Test, GasSnapshot, Deployers { using ProtocolFeeLibrary for uint24; @@ -176,43 +170,4 @@ contract ProtocolFeesTest is Test, GasSnapshot, Deployers { protocolFees.updateProtocolFees(currency0, amount); assertEq(protocolFees.protocolFeesAccrued(currency0), newAmount); } - - function test_fetchProtocolFee_succeeds() public { - protocolFees.setProtocolFeeController(feeController); - vm.prank(address(feeController)); - uint24 protocolFee = protocolFees.fetchProtocolFee(key); - assertEq(protocolFee, 0); - } - - function test_fetchProtocolFee_outOfBounds() public { - outOfBoundsFeeController = new OutOfBoundsProtocolFeeControllerTest(); - protocolFees.setProtocolFeeController(outOfBoundsFeeController); - vm.prank(address(outOfBoundsFeeController)); - uint24 protocolFee = protocolFees.fetchProtocolFee(key); - assertEq(protocolFee, 0); - } - - function test_fetchProtocolFee_overflowFee() public { - overflowFeeController = new OverflowProtocolFeeControllerTest(); - protocolFees.setProtocolFeeController(overflowFeeController); - vm.prank(address(overflowFeeController)); - uint24 protocolFee = protocolFees.fetchProtocolFee(key); - assertEq(protocolFee, 0); - } - - function test_fetchProtocolFee_invalidReturnSize() public { - invalidReturnSizeFeeController = new InvalidReturnSizeProtocolFeeControllerTest(); - protocolFees.setProtocolFeeController(invalidReturnSizeFeeController); - vm.prank(address(invalidReturnSizeFeeController)); - uint24 protocolFee = protocolFees.fetchProtocolFee(key); - assertEq(protocolFee, 0); - } - - function test_fetchProtocolFee_revert() public { - revertingFeeController = new RevertingProtocolFeeControllerTest(); - protocolFees.setProtocolFeeController(revertingFeeController); - vm.prank(address(revertingFeeController)); - uint24 protocolFee = protocolFees.fetchProtocolFee(key); - assertEq(protocolFee, 0); - } } diff --git a/test/libraries/BipsLibrary.t.sol b/test/libraries/BipsLibrary.t.sol deleted file mode 100644 index 02cc67d71..000000000 --- a/test/libraries/BipsLibrary.t.sol +++ /dev/null @@ -1,51 +0,0 @@ -// 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/libraries/Pool.t.sol b/test/libraries/Pool.t.sol index 4b8ec1e56..0ffa73f09 100644 --- a/test/libraries/Pool.t.sol +++ b/test/libraries/Pool.t.sol @@ -27,30 +27,27 @@ contract PoolTest is Test, GasSnapshot { uint24 constant MAX_PROTOCOL_FEE = ProtocolFeeLibrary.MAX_PROTOCOL_FEE; // 0.1% uint24 constant MAX_LP_FEE = LPFeeLibrary.MAX_LP_FEE; // 100% - function testPoolInitialize(uint160 sqrtPriceX96, uint24 protocolFee, uint24 swapFee) public { + function test_pool_initialize(uint160 sqrtPriceX96, uint24 swapFee) public { if (sqrtPriceX96 < TickMath.MIN_SQRT_PRICE || sqrtPriceX96 >= TickMath.MAX_SQRT_PRICE) { vm.expectRevert(abi.encodeWithSelector(TickMath.InvalidSqrtPrice.selector, sqrtPriceX96)); - state.initialize(sqrtPriceX96, protocolFee, swapFee); + state.initialize(sqrtPriceX96, swapFee); } else { - state.initialize(sqrtPriceX96, protocolFee, swapFee); + state.initialize(sqrtPriceX96, swapFee); assertEq(state.slot0.sqrtPriceX96(), sqrtPriceX96); - assertEq(state.slot0.protocolFee(), protocolFee); + assertEq(state.slot0.protocolFee(), 0); assertEq(state.slot0.tick(), TickMath.getTickAtSqrtPrice(sqrtPriceX96)); assertLt(state.slot0.tick(), TickMath.MAX_TICK); assertGt(state.slot0.tick(), TickMath.MIN_TICK - 1); } } - function testModifyLiquidity( - uint160 sqrtPriceX96, - uint24 protocolFee, - uint24 lpFee, - Pool.ModifyLiquidityParams memory params - ) public { + function test_modifyLiquidity(uint160 sqrtPriceX96, uint24 lpFee, Pool.ModifyLiquidityParams memory params) + public + { // Assumptions tested in PoolManager.t.sol params.tickSpacing = int24(bound(params.tickSpacing, TickMath.MIN_TICK_SPACING, TickMath.MAX_TICK_SPACING)); - testPoolInitialize(sqrtPriceX96, protocolFee, lpFee); + test_pool_initialize(sqrtPriceX96, lpFee); if (params.tickLower >= params.tickUpper) { vm.expectRevert(abi.encodeWithSelector(Pool.TicksMisordered.selector, params.tickLower, params.tickUpper)); @@ -106,9 +103,8 @@ contract PoolTest is Test, GasSnapshot { uint24 protocolFee = protocolFee1 << 12 | protocolFee0; // initialize and add liquidity - testModifyLiquidity( + test_modifyLiquidity( sqrtPriceX96, - protocolFee, lpFee, Pool.ModifyLiquidityParams({ owner: address(this), @@ -121,6 +117,10 @@ contract PoolTest is Test, GasSnapshot { ); Slot0 slot0 = state.slot0; + assertEq(slot0.protocolFee(), 0); + slot0 = slot0.setProtocolFee(protocolFee); + assertEq(slot0.protocolFee(), protocolFee); + uint24 _lpFee = params.lpFeeOverride.isOverride() ? params.lpFeeOverride.removeOverrideFlag() : lpFee; uint24 swapFee = protocolFee == 0 ? _lpFee : uint16(protocolFee).calculateSwapFee(_lpFee); diff --git a/test/utils/Deployers.sol b/test/utils/Deployers.sol index 9cf76b4a6..f15b42461 100644 --- a/test/utils/Deployers.sol +++ b/test/utils/Deployers.sol @@ -25,13 +25,7 @@ import {PoolClaimsTest} from "../../src/test/PoolClaimsTest.sol"; import {ActionsRouter} from "../../src/test/ActionsRouter.sol"; import {LiquidityAmounts} from "../../test/utils/LiquidityAmounts.sol"; import {StateLibrary} from "../../src/libraries/StateLibrary.sol"; -import { - ProtocolFeeControllerTest, - OutOfBoundsProtocolFeeControllerTest, - RevertingProtocolFeeControllerTest, - OverflowProtocolFeeControllerTest, - InvalidReturnSizeProtocolFeeControllerTest -} from "../../src/test/ProtocolFeeControllerTest.sol"; +import {ProtocolFeeControllerTest} from "../../src/test/ProtocolFeeControllerTest.sol"; contract Deployers { using LPFeeLibrary for uint24; @@ -70,10 +64,6 @@ contract Deployers { PoolClaimsTest claimsRouter; PoolNestedActionsTest nestedActionRouter; ProtocolFeeControllerTest feeController; - RevertingProtocolFeeControllerTest revertingFeeController; - OutOfBoundsProtocolFeeControllerTest outOfBoundsFeeController; - OverflowProtocolFeeControllerTest overflowFeeController; - InvalidReturnSizeProtocolFeeControllerTest invalidReturnSizeFeeController; PoolKey key; PoolKey nativeKey; @@ -99,10 +89,6 @@ contract Deployers { claimsRouter = new PoolClaimsTest(manager); nestedActionRouter = new PoolNestedActionsTest(manager); feeController = new ProtocolFeeControllerTest(); - revertingFeeController = new RevertingProtocolFeeControllerTest(); - outOfBoundsFeeController = new OutOfBoundsProtocolFeeControllerTest(); - overflowFeeController = new OverflowProtocolFeeControllerTest(); - invalidReturnSizeFeeController = new InvalidReturnSizeProtocolFeeControllerTest(); actionsRouter = new ActionsRouter(manager); manager.setProtocolFeeController(feeController);