From 9293e5ab1deed87e03c176d8af94b1af19eb3900 Mon Sep 17 00:00:00 2001 From: mr-uniswap <144828035+mr-uniswap@users.noreply.github.com> Date: Mon, 7 Oct 2024 09:04:41 +0900 Subject: [PATCH 1/9] fix: remove-quote (#893) --- .github/workflows/coverage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 01113933e..0bcf3c9de 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -32,7 +32,7 @@ jobs: run: | { echo 'COVERAGE<> "$GITHUB_OUTPUT" env: From 88482f7dc4356a7c395e199e454d50dc960bb6e7 Mon Sep 17 00:00:00 2001 From: saucepoint <98790946+saucepoint@users.noreply.github.com> Date: Tue, 8 Oct 2024 14:15:07 -0400 Subject: [PATCH 2/9] Remove initialize hookdata (#894) * remove hookData from initialize hooks * fix ABIs in tests * snapshots * forge fmt * foundryup'd gas snaps --- .../addLiquidity with empty hook.snap | 2 +- .forge-snapshots/initialize.snap | 2 +- .../poolManager bytecode size.snap | 2 +- .forge-snapshots/removeLiquidity CA fee.snap | 2 +- .../removeLiquidity with empty hook.snap | 2 +- .../swap CA custom curve + swap noop.snap | 2 +- .../swap CA fee on unspecified.snap | 2 +- ...wap skips hook call if hook is caller.snap | 2 +- .forge-snapshots/swap with hooks.snap | 2 +- .../swap with lp fee and protocol fee.snap | 2 +- .../swap with return dynamic fee.snap | 2 +- .../update dynamic fee in before swap.snap | 2 +- src/PoolManager.sol | 10 +--- src/interfaces/IHooks.sol | 16 ++---- src/interfaces/IPoolManager.sol | 5 +- src/libraries/Hooks.sol | 11 ++-- src/test/BaseTestHooks.sol | 14 ++--- src/test/DynamicFeesTestHook.sol | 6 +- src/test/EmptyTestHooks.sol | 14 +---- src/test/MockHooks.sol | 16 ++---- src/test/PoolNestedActionsTest.sol | 2 +- src/test/ProxyPoolManager.sol | 10 +--- src/test/SkipCallsTestHook.sol | 16 ++---- test/CustomAccounting.t.sol | 2 +- test/DynamicFees.t.sol | 17 ++---- test/DynamicReturnFees.t.sol | 15 ++--- test/ModifyLiquidity.t.sol | 12 ++-- test/PoolManager.swap.t.sol | 2 +- test/PoolManager.t.sol | 37 ++++++------ test/PoolManagerInitialize.t.sol | 56 +++++++++---------- test/SkipCallsTestHook.t.sol | 2 +- test/Sync.t.sol | 2 +- test/libraries/Hooks.t.sol | 10 ++-- test/libraries/StateLibrary.t.sol | 2 +- test/utils/Deployers.sol | 34 +++++------ 35 files changed, 131 insertions(+), 204 deletions(-) diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap index ec66eb5f5..b04e884a5 100644 --- a/.forge-snapshots/addLiquidity with empty hook.snap +++ b/.forge-snapshots/addLiquidity with empty hook.snap @@ -1 +1 @@ -274206 \ No newline at end of file +274240 \ No newline at end of file diff --git a/.forge-snapshots/initialize.snap b/.forge-snapshots/initialize.snap index 3568d3ea6..57a866726 100644 --- a/.forge-snapshots/initialize.snap +++ b/.forge-snapshots/initialize.snap @@ -1 +1 @@ -60013 \ No newline at end of file +59536 \ No newline at end of file diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap index e085ea34a..2666adbeb 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -24073 \ No newline at end of file +24001 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity CA fee.snap b/.forge-snapshots/removeLiquidity CA fee.snap index 141f3ac98..5b9da2ead 100644 --- a/.forge-snapshots/removeLiquidity CA fee.snap +++ b/.forge-snapshots/removeLiquidity CA fee.snap @@ -1 +1 @@ -141217 \ No newline at end of file +141195 \ 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 b9000fe01..12528ddad 100644 --- a/.forge-snapshots/removeLiquidity with empty hook.snap +++ b/.forge-snapshots/removeLiquidity with empty hook.snap @@ -1 +1 @@ -130585 \ No newline at end of file +130597 \ 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 1cbd474c9..c4cdede27 100644 --- a/.forge-snapshots/swap CA custom curve + swap noop.snap +++ b/.forge-snapshots/swap CA custom curve + swap noop.snap @@ -1 +1 @@ -124651 \ No newline at end of file +124629 \ 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 6cc8394ff..8f0ca3957 100644 --- a/.forge-snapshots/swap CA fee on unspecified.snap +++ b/.forge-snapshots/swap CA fee on unspecified.snap @@ -1 +1 @@ -154771 \ No newline at end of file +154749 \ 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 2d821ebed..a98a52ade 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 @@ -206437 \ No newline at end of file +206415 \ No newline at end of file diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap index d5d13a6c4..f6ae0682c 100644 --- a/.forge-snapshots/swap with hooks.snap +++ b/.forge-snapshots/swap with hooks.snap @@ -1 +1 @@ -132331 \ No newline at end of file +132321 \ No newline at end of file diff --git a/.forge-snapshots/swap with lp fee and protocol fee.snap b/.forge-snapshots/swap with lp fee and protocol fee.snap index 99b68fd83..8c298d1fe 100644 --- a/.forge-snapshots/swap with lp fee and protocol fee.snap +++ b/.forge-snapshots/swap with lp fee and protocol fee.snap @@ -1 +1 @@ -169615 \ No newline at end of file +169593 \ 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 fb03eaeca..09bc9151b 100644 --- a/.forge-snapshots/swap with return dynamic fee.snap +++ b/.forge-snapshots/swap with return dynamic fee.snap @@ -1 +1 @@ -145695 \ No newline at end of file +145673 \ No newline at end of file diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap index 195a70586..8a6f38517 100644 --- a/.forge-snapshots/update dynamic fee in before swap.snap +++ b/.forge-snapshots/update dynamic fee in before swap.snap @@ -1 +1 @@ -147978 \ No newline at end of file +147956 \ No newline at end of file diff --git a/src/PoolManager.sol b/src/PoolManager.sol index f366d33aa..f7d4b529f 100644 --- a/src/PoolManager.sol +++ b/src/PoolManager.sol @@ -112,11 +112,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim } /// @inheritdoc IPoolManager - function initialize(PoolKey memory key, uint160 sqrtPriceX96, bytes calldata hookData) - external - noDelegateCall - returns (int24 tick) - { + function initialize(PoolKey memory key, uint160 sqrtPriceX96) external noDelegateCall returns (int24 tick) { // see TickBitmap.sol for overflow conditions that can arise from tick spacing being too large if (key.tickSpacing > MAX_TICK_SPACING) TickSpacingTooLarge.selector.revertWith(key.tickSpacing); if (key.tickSpacing < MIN_TICK_SPACING) TickSpacingTooSmall.selector.revertWith(key.tickSpacing); @@ -129,14 +125,14 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim uint24 lpFee = key.fee.getInitialLPFee(); - key.hooks.beforeInitialize(key, sqrtPriceX96, hookData); + key.hooks.beforeInitialize(key, sqrtPriceX96); PoolId id = key.toId(); uint24 protocolFee = _fetchProtocolFee(key); tick = _pools[id].initialize(sqrtPriceX96, protocolFee, lpFee); - key.hooks.afterInitialize(key, sqrtPriceX96, tick, hookData); + key.hooks.afterInitialize(key, sqrtPriceX96, tick); // emit all details of a pool key. poolkeys are not saved in storage and must always be provided by the caller // the key's fee may be a static fee or a sentinel to denote a dynamic fee. diff --git a/src/interfaces/IHooks.sol b/src/interfaces/IHooks.sol index e0d4a1f4e..a541e2681 100644 --- a/src/interfaces/IHooks.sol +++ b/src/interfaces/IHooks.sol @@ -17,26 +17,18 @@ interface IHooks { /// @param sender The initial msg.sender for the initialize call /// @param key The key for the pool being initialized /// @param sqrtPriceX96 The sqrt(price) of the pool as a Q64.96 - /// @param hookData Arbitrary data handed into the PoolManager by the initializer to be be passed on to the hook /// @return bytes4 The function selector for the hook - function beforeInitialize(address sender, PoolKey calldata key, uint160 sqrtPriceX96, bytes calldata hookData) - external - returns (bytes4); + function beforeInitialize(address sender, PoolKey calldata key, uint160 sqrtPriceX96) external returns (bytes4); /// @notice The hook called after the state of a pool is initialized /// @param sender The initial msg.sender for the initialize call /// @param key The key for the pool being initialized /// @param sqrtPriceX96 The sqrt(price) of the pool as a Q64.96 /// @param tick The current tick after the state of a pool is initialized - /// @param hookData Arbitrary data handed into the PoolManager by the initializer to be be passed on to the hook /// @return bytes4 The function selector for the hook - function afterInitialize( - address sender, - PoolKey calldata key, - uint160 sqrtPriceX96, - int24 tick, - bytes calldata hookData - ) external returns (bytes4); + function afterInitialize(address sender, PoolKey calldata key, uint160 sqrtPriceX96, int24 tick) + external + returns (bytes4); /// @notice The hook called before liquidity is added /// @param sender The initial msg.sender for the add liquidity call diff --git a/src/interfaces/IPoolManager.sol b/src/interfaces/IPoolManager.sol index dc5b60b49..3269851d0 100644 --- a/src/interfaces/IPoolManager.sol +++ b/src/interfaces/IPoolManager.sol @@ -116,11 +116,8 @@ interface IPoolManager is IProtocolFees, IERC6909Claims, IExtsload, IExttload { /// @dev A swap fee totaling MAX_SWAP_FEE (100%) makes exact output swaps impossible since the input is entirely consumed by the fee /// @param key The pool key for the pool to initialize /// @param sqrtPriceX96 The initial square root price - /// @param hookData The data to pass through to the initialize hooks /// @return tick The initial tick of the pool - function initialize(PoolKey memory key, uint160 sqrtPriceX96, bytes calldata hookData) - external - returns (int24 tick); + function initialize(PoolKey memory key, uint160 sqrtPriceX96) external returns (int24 tick); struct ModifyLiquidityParams { // the lower and upper tick of the position diff --git a/src/libraries/Hooks.sol b/src/libraries/Hooks.sol index 28036e4f0..e175d7bf5 100644 --- a/src/libraries/Hooks.sol +++ b/src/libraries/Hooks.sol @@ -175,22 +175,19 @@ library Hooks { } /// @notice calls beforeInitialize hook if permissioned and validates return value - function beforeInitialize(IHooks self, PoolKey memory key, uint160 sqrtPriceX96, bytes calldata hookData) - internal - noSelfCall(self) - { + function beforeInitialize(IHooks self, PoolKey memory key, uint160 sqrtPriceX96) internal noSelfCall(self) { if (self.hasPermission(BEFORE_INITIALIZE_FLAG)) { - self.callHook(abi.encodeCall(IHooks.beforeInitialize, (msg.sender, key, sqrtPriceX96, hookData))); + self.callHook(abi.encodeCall(IHooks.beforeInitialize, (msg.sender, key, sqrtPriceX96))); } } /// @notice calls afterInitialize hook if permissioned and validates return value - function afterInitialize(IHooks self, PoolKey memory key, uint160 sqrtPriceX96, int24 tick, bytes calldata hookData) + function afterInitialize(IHooks self, PoolKey memory key, uint160 sqrtPriceX96, int24 tick) internal noSelfCall(self) { if (self.hasPermission(AFTER_INITIALIZE_FLAG)) { - self.callHook(abi.encodeCall(IHooks.afterInitialize, (msg.sender, key, sqrtPriceX96, tick, hookData))); + self.callHook(abi.encodeCall(IHooks.afterInitialize, (msg.sender, key, sqrtPriceX96, tick))); } } diff --git a/src/test/BaseTestHooks.sol b/src/test/BaseTestHooks.sol index f4d673f06..83c9f9af9 100644 --- a/src/test/BaseTestHooks.sol +++ b/src/test/BaseTestHooks.sol @@ -10,12 +10,11 @@ import {IPoolManager} from "../interfaces/IPoolManager.sol"; contract BaseTestHooks is IHooks { error HookNotImplemented(); - function beforeInitialize( - address, /* sender **/ - PoolKey calldata, /* key **/ - uint160, /* sqrtPriceX96 **/ - bytes calldata /* hookData **/ - ) external virtual returns (bytes4) { + function beforeInitialize(address, /* sender **/ PoolKey calldata, /* key **/ uint160 /* sqrtPriceX96 **/ ) + external + virtual + returns (bytes4) + { revert HookNotImplemented(); } @@ -23,8 +22,7 @@ contract BaseTestHooks is IHooks { address, /* sender **/ PoolKey calldata, /* key **/ uint160, /* sqrtPriceX96 **/ - int24, /* tick **/ - bytes calldata /* hookData **/ + int24 /* tick **/ ) external virtual returns (bytes4) { revert HookNotImplemented(); } diff --git a/src/test/DynamicFeesTestHook.sol b/src/test/DynamicFeesTestHook.sol index 7d6aa0e5a..8b546d09a 100644 --- a/src/test/DynamicFeesTestHook.sol +++ b/src/test/DynamicFeesTestHook.sol @@ -19,11 +19,7 @@ contract DynamicFeesTestHook is BaseTestHooks { fee = _fee; } - function afterInitialize(address, PoolKey calldata key, uint160, int24, bytes calldata) - external - override - returns (bytes4) - { + function afterInitialize(address, PoolKey calldata key, uint160, int24) external override returns (bytes4) { manager.updateDynamicLPFee(key, fee); return IHooks.afterInitialize.selector; } diff --git a/src/test/EmptyTestHooks.sol b/src/test/EmptyTestHooks.sol index 51f28c12c..43544cf90 100644 --- a/src/test/EmptyTestHooks.sol +++ b/src/test/EmptyTestHooks.sol @@ -32,21 +32,11 @@ contract EmptyTestHooks is IHooks { ); } - function beforeInitialize(address, PoolKey calldata, uint160, bytes calldata) - external - pure - override - returns (bytes4) - { + function beforeInitialize(address, PoolKey calldata, uint160) external pure override returns (bytes4) { return IHooks.beforeInitialize.selector; } - function afterInitialize(address, PoolKey calldata, uint160, int24, bytes calldata) - external - pure - override - returns (bytes4) - { + function afterInitialize(address, PoolKey calldata, uint160, int24) external pure override returns (bytes4) { return IHooks.afterInitialize.selector; } diff --git a/src/test/MockHooks.sol b/src/test/MockHooks.sol index 08ef4e79e..8299ddfc8 100644 --- a/src/test/MockHooks.sol +++ b/src/test/MockHooks.sol @@ -27,22 +27,14 @@ contract MockHooks is IHooks { mapping(PoolId => uint16) public lpFees; - function beforeInitialize(address, PoolKey calldata, uint160, bytes calldata hookData) - external - override - returns (bytes4) - { - beforeInitializeData = hookData; + function beforeInitialize(address, PoolKey calldata, uint160) external override returns (bytes4) { + beforeInitializeData = new bytes(123); bytes4 selector = MockHooks.beforeInitialize.selector; return returnValues[selector] == bytes4(0) ? selector : returnValues[selector]; } - function afterInitialize(address, PoolKey calldata, uint160, int24, bytes calldata hookData) - external - override - returns (bytes4) - { - afterInitializeData = hookData; + function afterInitialize(address, PoolKey calldata, uint160, int24) external override returns (bytes4) { + afterInitializeData = new bytes(123); bytes4 selector = MockHooks.afterInitialize.selector; return returnValues[selector] == bytes4(0) ? selector : returnValues[selector]; } diff --git a/src/test/PoolNestedActionsTest.sol b/src/test/PoolNestedActionsTest.sol index 11f8996bf..044ca0b71 100644 --- a/src/test/PoolNestedActionsTest.sol +++ b/src/test/PoolNestedActionsTest.sol @@ -223,7 +223,7 @@ contract NestedActionExecutor is Test, PoolTestBase { PoolId id = key.toId(); (uint256 price,,,) = manager.getSlot0(id); assertEq(price, 0); - manager.initialize(key, Constants.SQRT_PRICE_1_2, Constants.ZERO_BYTES); + manager.initialize(key, Constants.SQRT_PRICE_1_2); (price,,,) = manager.getSlot0(id); assertEq(price, Constants.SQRT_PRICE_1_2); } diff --git a/src/test/ProxyPoolManager.sol b/src/test/ProxyPoolManager.sol index b82bf8428..1d9565c3f 100644 --- a/src/test/ProxyPoolManager.sol +++ b/src/test/ProxyPoolManager.sol @@ -68,11 +68,7 @@ contract ProxyPoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909 } /// @inheritdoc IPoolManager - function initialize(PoolKey memory key, uint160 sqrtPriceX96, bytes calldata hookData) - external - noDelegateCall - returns (int24 tick) - { + function initialize(PoolKey memory key, uint160 sqrtPriceX96) external noDelegateCall returns (int24 tick) { // see TickBitmap.sol for overflow conditions that can arise from tick spacing being too large if (key.tickSpacing > MAX_TICK_SPACING) TickSpacingTooLarge.selector.revertWith(key.tickSpacing); if (key.tickSpacing < MIN_TICK_SPACING) TickSpacingTooSmall.selector.revertWith(key.tickSpacing); @@ -85,14 +81,14 @@ contract ProxyPoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909 uint24 lpFee = key.fee.getInitialLPFee(); - key.hooks.beforeInitialize(key, sqrtPriceX96, hookData); + key.hooks.beforeInitialize(key, sqrtPriceX96); PoolId id = key.toId(); uint24 protocolFee = _fetchProtocolFee(key); tick = _pools[id].initialize(sqrtPriceX96, protocolFee, lpFee); - key.hooks.afterInitialize(key, sqrtPriceX96, tick, hookData); + key.hooks.afterInitialize(key, sqrtPriceX96, tick); // emit all details of a pool key. poolkeys are not saved in storage and must always be provided by the caller // the key's fee may be a static fee or a sentinel to denote a dynamic fee. diff --git a/src/test/SkipCallsTestHook.sol b/src/test/SkipCallsTestHook.sol index 341709fc4..d6021bdf9 100644 --- a/src/test/SkipCallsTestHook.sol +++ b/src/test/SkipCallsTestHook.sol @@ -27,23 +27,19 @@ contract SkipCallsTestHook is BaseTestHooks, Test { manager = _manager; } - function beforeInitialize(address, PoolKey calldata key, uint160 sqrtPriceX96, bytes calldata hookData) - external - override - returns (bytes4) - { + function beforeInitialize(address, PoolKey calldata key, uint160 sqrtPriceX96) external override returns (bytes4) { counter++; - _initialize(key, sqrtPriceX96, hookData); + _initialize(key, sqrtPriceX96); return IHooks.beforeInitialize.selector; } - function afterInitialize(address, PoolKey calldata key, uint160 sqrtPriceX96, int24, bytes calldata hookData) + function afterInitialize(address, PoolKey calldata key, uint160 sqrtPriceX96, int24) external override returns (bytes4) { counter++; - _initialize(key, sqrtPriceX96, hookData); + _initialize(key, sqrtPriceX96); return IHooks.afterInitialize.selector; } @@ -137,10 +133,10 @@ contract SkipCallsTestHook is BaseTestHooks, Test { return IHooks.afterDonate.selector; } - function _initialize(PoolKey memory key, uint160 sqrtPriceX96, bytes calldata hookData) public { + function _initialize(PoolKey memory key, uint160 sqrtPriceX96) public { // initialize a new pool with different fee key.fee = 2000; - IPoolManager(manager).initialize(key, sqrtPriceX96, hookData); + IPoolManager(manager).initialize(key, sqrtPriceX96); } function _swap(PoolKey calldata key, IPoolManager.SwapParams memory params, bytes calldata hookData) public { diff --git a/test/CustomAccounting.t.sol b/test/CustomAccounting.t.sol index 6cb3d0bb9..e4942dce3 100644 --- a/test/CustomAccounting.t.sol +++ b/test/CustomAccounting.t.sol @@ -65,7 +65,7 @@ contract CustomAccountingTest is Test, Deployers, GasSnapshot { vm.etch(hookAddr, implAddr.code); hook = hookAddr; - (key,) = initPoolAndAddLiquidity(currency0, currency1, IHooks(hookAddr), 100, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPoolAndAddLiquidity(currency0, currency1, IHooks(hookAddr), 100, SQRT_PRICE_1_1); } // ------------------------ SWAP ------------------------ diff --git a/test/DynamicFees.t.sol b/test/DynamicFees.t.sol index 367a3adbb..cd8a3b332 100644 --- a/test/DynamicFees.t.sol +++ b/test/DynamicFees.t.sol @@ -58,12 +58,7 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot { deployMintAndApprove2Currencies(); (key,) = initPoolAndAddLiquidity( - currency0, - currency1, - IHooks(address(dynamicFeesHooks)), - LPFeeLibrary.DYNAMIC_FEE_FLAG, - SQRT_PRICE_1_1, - ZERO_BYTES + currency0, currency1, IHooks(address(dynamicFeesHooks)), LPFeeLibrary.DYNAMIC_FEE_FLAG, SQRT_PRICE_1_1 ); } @@ -79,7 +74,7 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot { abi.encodeWithSelector(LPFeeLibrary.LPFeeTooLarge.selector, fee) ) ); - manager.initialize(key, SQRT_PRICE_1_1, ZERO_BYTES); + manager.initialize(key, SQRT_PRICE_1_1); } function test_initialize_initializesFeeTo0() public { @@ -88,7 +83,7 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot { // this fee is not fetched as theres no afterInitialize hook dynamicFeesNoHooks.setFee(1000000); - manager.initialize(key, SQRT_PRICE_1_1, ZERO_BYTES); + manager.initialize(key, SQRT_PRICE_1_1); assertEq(_fetchPoolLPFee(key), 0); } @@ -96,7 +91,7 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot { key.tickSpacing = 30; dynamicFeesHooks.setFee(123); - manager.initialize(key, SQRT_PRICE_1_1, ZERO_BYTES); + manager.initialize(key, SQRT_PRICE_1_1); assertEq(_fetchPoolLPFee(key), 123); } @@ -117,7 +112,7 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot { abi.encodeWithSelector(IPoolManager.UnauthorizedDynamicLPFeeUpdate.selector) ) ); - manager.initialize(key, SQRT_PRICE_1_1, ZERO_BYTES); + manager.initialize(key, SQRT_PRICE_1_1); } function test_updateDynamicLPFee_beforeSwap_failsWithTooLargeFee() public { @@ -318,7 +313,7 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot { function test_swap_withDynamicFee_gas() public { (key,) = initPoolAndAddLiquidity( - currency0, currency1, dynamicFeesNoHooks, LPFeeLibrary.DYNAMIC_FEE_FLAG, SQRT_PRICE_1_1, ZERO_BYTES + currency0, currency1, dynamicFeesNoHooks, LPFeeLibrary.DYNAMIC_FEE_FLAG, SQRT_PRICE_1_1 ); assertEq(_fetchPoolLPFee(key), 0); diff --git a/test/DynamicReturnFees.t.sol b/test/DynamicReturnFees.t.sol index 768d53c8b..4975e804d 100644 --- a/test/DynamicReturnFees.t.sol +++ b/test/DynamicReturnFees.t.sol @@ -49,12 +49,7 @@ contract TestDynamicReturnFees is Test, Deployers, GasSnapshot { deployMintAndApprove2Currencies(); (key,) = initPoolAndAddLiquidity( - currency0, - currency1, - IHooks(address(dynamicReturnFeesHook)), - LPFeeLibrary.DYNAMIC_FEE_FLAG, - SQRT_PRICE_1_1, - ZERO_BYTES + currency0, currency1, IHooks(address(dynamicReturnFeesHook)), LPFeeLibrary.DYNAMIC_FEE_FLAG, SQRT_PRICE_1_1 ); } @@ -107,7 +102,7 @@ contract TestDynamicReturnFees is Test, Deployers, GasSnapshot { function test_dynamicReturnSwapFee_initializeZeroSwapFee() public { key.tickSpacing = 30; - manager.initialize(key, SQRT_PRICE_1_1, ZERO_BYTES); + manager.initialize(key, SQRT_PRICE_1_1); assertEq(_fetchPoolSwapFee(key), 0); } @@ -115,9 +110,7 @@ contract TestDynamicReturnFees is Test, Deployers, GasSnapshot { key.fee = 3000; // static fee dynamicReturnFeesHook.setFee(1000); // 0.10% fee is NOT used because the pool has a static fee - initPoolAndAddLiquidity( - currency0, currency1, IHooks(address(dynamicReturnFeesHook)), 3000, SQRT_PRICE_1_1, ZERO_BYTES - ); + initPoolAndAddLiquidity(currency0, currency1, IHooks(address(dynamicReturnFeesHook)), 3000, SQRT_PRICE_1_1); assertEq(_fetchPoolSwapFee(key), 3000); // despite returning a valid swap fee (1000), the static fee is used @@ -136,7 +129,7 @@ contract TestDynamicReturnFees is Test, Deployers, GasSnapshot { // create a new pool with an initial fee of 123 key.tickSpacing = 30; - manager.initialize(key, SQRT_PRICE_1_1, ZERO_BYTES); + manager.initialize(key, SQRT_PRICE_1_1); modifyLiquidityRouter.modifyLiquidity(key, LIQUIDITY_PARAMS, ZERO_BYTES); uint24 initialFee = 123; dynamicReturnFeesHook.forcePoolFeeUpdate(key, initialFee); diff --git a/test/ModifyLiquidity.t.sol b/test/ModifyLiquidity.t.sol index dc8094091..f4ebe74aa 100644 --- a/test/ModifyLiquidity.t.sol +++ b/test/ModifyLiquidity.t.sol @@ -40,7 +40,7 @@ contract ModifyLiquidityTest is Test, Logger, Deployers, JavascriptFfi, Fuzzers, function setUp() public { deployFreshManagerAndRouters(); deployMintAndApprove2Currencies(); - (simpleKey, simplePoolId) = initPool(currency0, currency1, IHooks(address(0)), 3000, SQRT_PRICE_1_1, ZERO_BYTES); + (simpleKey, simplePoolId) = initPool(currency0, currency1, IHooks(address(0)), 3000, SQRT_PRICE_1_1); } /*////////////////////////////////////////////////////////////// @@ -72,9 +72,8 @@ contract ModifyLiquidityTest is Test, Logger, Deployers, JavascriptFfi, Fuzzers, // Static edge case, no fuzz test, to make sure we test max tickspacing. function test_ffi_addLiqudity_weirdPool_0_returnsCorrectLiquidityDelta() public { // Use a pool with TickSpacing of MAX_TICK_SPACING - (PoolKey memory wp0, PoolId wpId0) = initPool( - currency0, currency1, IHooks(address(0)), 500, TickMath.MAX_TICK_SPACING, SQRT_PRICE_1_1, ZERO_BYTES - ); + (PoolKey memory wp0, PoolId wpId0) = + initPool(currency0, currency1, IHooks(address(0)), 500, TickMath.MAX_TICK_SPACING, SQRT_PRICE_1_1); // Set the params to add random amount of liquidity to random tick boundary. int24 tickUpper = TickMath.MAX_TICK_SPACING * 4; @@ -97,9 +96,8 @@ contract ModifyLiquidityTest is Test, Logger, Deployers, JavascriptFfi, Fuzzers, // Static edge case, no fuzz test, to make sure we test min tick spacing. function test_ffi_addLiqudity_weirdPool_1_returnsCorrectLiquidityDelta() public { // Use a pool with TickSpacing of MIN_TICK_SPACING - (PoolKey memory wp0, PoolId wpId0) = initPool( - currency0, currency1, IHooks(address(0)), 551, TickMath.MIN_TICK_SPACING, SQRT_PRICE_1_1, ZERO_BYTES - ); + (PoolKey memory wp0, PoolId wpId0) = + initPool(currency0, currency1, IHooks(address(0)), 551, TickMath.MIN_TICK_SPACING, SQRT_PRICE_1_1); // Set the params to add random amount of liquidity to random tick boundary. int24 tickUpper = TickMath.MIN_TICK_SPACING * 17; diff --git a/test/PoolManager.swap.t.sol b/test/PoolManager.swap.t.sol index 30c9bc66a..1a7e1ebd3 100644 --- a/test/PoolManager.swap.t.sol +++ b/test/PoolManager.swap.t.sol @@ -41,7 +41,7 @@ abstract contract V3Fuzzer is V3Helper, Deployers, Fuzzers, IUniswapV3MintCallba v3Pool.initialize(sqrtPriceX96); key_ = PoolKey(currency0, currency1, fee, tickSpacing, IHooks(address(0))); - manager.initialize(key_, sqrtPriceX96, ""); + manager.initialize(key_, sqrtPriceX96); } function addLiquidity( diff --git a/test/PoolManager.t.sol b/test/PoolManager.t.sol index cb8c8fa73..be71725a1 100644 --- a/test/PoolManager.t.sol +++ b/test/PoolManager.t.sol @@ -179,7 +179,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { MockContract(mockAddr).setImplementation(hookAddr); - (key,) = initPool(currency0, currency1, IHooks(mockAddr), 3000, sqrtPriceX96, ZERO_BYTES); + (key,) = initPool(currency0, currency1, IHooks(mockAddr), 3000, sqrtPriceX96); BalanceDelta balanceDelta = modifyLiquidityRouter.modifyLiquidity(key, LIQUIDITY_PARAMS, ZERO_BYTES); @@ -214,7 +214,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { MockContract(mockAddr).setImplementation(hookAddr); - (key,) = initPool(currency0, currency1, IHooks(mockAddr), 3000, sqrtPriceX96, ZERO_BYTES); + (key,) = initPool(currency0, currency1, IHooks(mockAddr), 3000, sqrtPriceX96); modifyLiquidityRouter.modifyLiquidity(key, LIQUIDITY_PARAMS, ZERO_BYTES); BalanceDelta balanceDelta = modifyLiquidityRouter.modifyLiquidity(key, REMOVE_LIQUIDITY_PARAMS, ZERO_BYTES); @@ -243,7 +243,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { vm.etch(hookAddr, address(impl).code); MockHooks mockHooks = MockHooks(hookAddr); - (key,) = initPool(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPool(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1); mockHooks.setReturnValue(mockHooks.beforeAddLiquidity.selector, bytes4(0xdeadbeef)); mockHooks.setReturnValue(mockHooks.afterAddLiquidity.selector, bytes4(0xdeadbeef)); @@ -265,7 +265,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { vm.etch(hookAddr, address(impl).code); MockHooks mockHooks = MockHooks(hookAddr); - (key,) = initPool(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPool(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1); modifyLiquidityRouter.modifyLiquidity(key, LIQUIDITY_PARAMS, ZERO_BYTES); mockHooks.setReturnValue(mockHooks.beforeRemoveLiquidity.selector, bytes4(0xdeadbeef)); @@ -288,7 +288,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { vm.etch(hookAddr, address(impl).code); MockHooks mockHooks = MockHooks(hookAddr); - (key,) = initPool(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPool(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1); mockHooks.setReturnValue(mockHooks.beforeAddLiquidity.selector, mockHooks.beforeAddLiquidity.selector); mockHooks.setReturnValue(mockHooks.afterAddLiquidity.selector, mockHooks.afterAddLiquidity.selector); @@ -313,7 +313,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { vm.etch(hookAddr, address(impl).code); MockHooks mockHooks = MockHooks(hookAddr); - (key,) = initPool(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPool(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1); modifyLiquidityRouter.modifyLiquidity(key, LIQUIDITY_PARAMS, ZERO_BYTES); mockHooks.setReturnValue(mockHooks.beforeRemoveLiquidity.selector, mockHooks.beforeRemoveLiquidity.selector); @@ -449,7 +449,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { vm.etch(allHooksAddr, address(impl).code); MockHooks mockHooks = MockHooks(allHooksAddr); - (key,) = initPool(currency0, currency1, mockHooks, 3000, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPool(currency0, currency1, mockHooks, 3000, SQRT_PRICE_1_1); modifyLiquidityRouter.modifyLiquidity(key, LIQUIDITY_PARAMS, ZERO_BYTES); snapLastCall("addLiquidity with empty hook"); @@ -461,7 +461,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { vm.etch(allHooksAddr, address(impl).code); MockHooks mockHooks = MockHooks(allHooksAddr); - (key,) = initPool(currency0, currency1, mockHooks, 3000, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPool(currency0, currency1, mockHooks, 3000, SQRT_PRICE_1_1); modifyLiquidityRouter.modifyLiquidity(key, LIQUIDITY_PARAMS, ZERO_BYTES); modifyLiquidityRouter.modifyLiquidity(key, REMOVE_LIQUIDITY_PARAMS, ZERO_BYTES); @@ -528,7 +528,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { MockContract(mockAddr).setImplementation(hookAddr); - (key,) = initPoolAndAddLiquidity(currency0, currency1, IHooks(mockAddr), 3000, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPoolAndAddLiquidity(currency0, currency1, IHooks(mockAddr), 3000, SQRT_PRICE_1_1); PoolSwapTest.TestSettings memory testSettings = PoolSwapTest.TestSettings({takeClaims: true, settleUsingBurn: false}); @@ -554,7 +554,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { vm.etch(hookAddr, address(impl).code); MockHooks mockHooks = MockHooks(hookAddr); - (key,) = initPoolAndAddLiquidity(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPoolAndAddLiquidity(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1); IPoolManager.SwapParams memory swapParams = IPoolManager.SwapParams({zeroForOne: true, amountSpecified: 10, sqrtPriceLimitX96: SQRT_PRICE_1_2}); @@ -582,7 +582,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { vm.etch(hookAddr, address(impl).code); MockHooks mockHooks = MockHooks(hookAddr); - (key,) = initPoolAndAddLiquidity(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPoolAndAddLiquidity(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1); IPoolManager.SwapParams memory swapParams = IPoolManager.SwapParams({zeroForOne: true, amountSpecified: -10, sqrtPriceLimitX96: SQRT_PRICE_1_2}); @@ -630,7 +630,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { vm.etch(allHooksAddr, address(impl).code); MockHooks mockHooks = MockHooks(allHooksAddr); - (key,) = initPoolAndAddLiquidity(currency0, currency1, mockHooks, 3000, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPoolAndAddLiquidity(currency0, currency1, mockHooks, 3000, SQRT_PRICE_1_1); PoolSwapTest.TestSettings memory testSettings = PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false}); @@ -821,7 +821,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { function test_donate_failsIfNoLiquidity(uint160 sqrtPriceX96) public { sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_PRICE, TickMath.MAX_SQRT_PRICE - 1)); - (key,) = initPool(currency0, currency1, IHooks(address(0)), 100, sqrtPriceX96, ZERO_BYTES); + (key,) = initPool(currency0, currency1, IHooks(address(0)), 100, sqrtPriceX96); vm.expectRevert(Pool.NoLiquidityToReceiveFees.selector); donateRouter.donate(key, 100, 100, ZERO_BYTES); @@ -860,7 +860,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { vm.etch(hookAddr, address(impl).code); MockHooks mockHooks = MockHooks(hookAddr); - (key,) = initPoolAndAddLiquidity(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPoolAndAddLiquidity(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1); mockHooks.setReturnValue(mockHooks.beforeDonate.selector, bytes4(0xdeadbeef)); mockHooks.setReturnValue(mockHooks.afterDonate.selector, bytes4(0xdeadbeef)); @@ -882,7 +882,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { vm.etch(hookAddr, address(impl).code); MockHooks mockHooks = MockHooks(hookAddr); - (key,) = initPoolAndAddLiquidity(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPoolAndAddLiquidity(currency0, currency1, mockHooks, 100, SQRT_PRICE_1_1); mockHooks.setReturnValue(mockHooks.beforeDonate.selector, mockHooks.beforeDonate.selector); mockHooks.setReturnValue(mockHooks.afterDonate.selector, mockHooks.afterDonate.selector); @@ -924,8 +924,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { (currency0Invalid ? currency0 : invalidCurrency), IHooks(address(0)), 3000, - SQRT_PRICE_1_1, - ZERO_BYTES + SQRT_PRICE_1_1 ); (uint256 amount0, uint256 amount1) = currency0Invalid ? (1, 0) : (0, 1); @@ -1155,7 +1154,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { // hooks: IHooks(address(0)), // tickSpacing: 10 // }); - // manager.initialize(key, SQRT_PRICE_1_1, ZERO_BYTES); + // manager.initialize(key, SQRT_PRICE_1_1); // PoolId poolId = key.toId(); // bytes32 slot0Bytes = manager.extsload(keccak256(abi.encode(poolId, POOL_SLOT))); @@ -1179,7 +1178,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { // hooks: IHooks(address(0)), // tickSpacing: 10 // }); - // manager.initialize(key, SQRT_PRICE_1_1, ZERO_BYTES); + // manager.initialize(key, SQRT_PRICE_1_1); // // populate feeGrowthGlobalX128 struct w/ modify + swap // modifyLiquidityRouter.modifyLiquidity(key, IPoolManager.ModifyLiquidityParams(-120, 120, 5 ether, 0)); diff --git a/test/PoolManagerInitialize.t.sol b/test/PoolManagerInitialize.t.sol index 22478c516..39632f6a3 100644 --- a/test/PoolManagerInitialize.t.sol +++ b/test/PoolManagerInitialize.t.sol @@ -63,30 +63,30 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { if (key0.tickSpacing > TickMath.MAX_TICK_SPACING) { vm.expectRevert(abi.encodeWithSelector(IPoolManager.TickSpacingTooLarge.selector, key0.tickSpacing)); - manager.initialize(key0, sqrtPriceX96, ZERO_BYTES); + manager.initialize(key0, sqrtPriceX96); } else if (key0.tickSpacing < TickMath.MIN_TICK_SPACING) { vm.expectRevert(abi.encodeWithSelector(IPoolManager.TickSpacingTooSmall.selector, key0.tickSpacing)); - manager.initialize(key0, sqrtPriceX96, ZERO_BYTES); + manager.initialize(key0, sqrtPriceX96); } else if (key0.currency0 >= key0.currency1) { vm.expectRevert( abi.encodeWithSelector( IPoolManager.CurrenciesOutOfOrderOrEqual.selector, key0.currency0, key0.currency1 ) ); - manager.initialize(key0, sqrtPriceX96, ZERO_BYTES); + manager.initialize(key0, sqrtPriceX96); } else if (!key0.hooks.isValidHookAddress(key0.fee)) { vm.expectRevert(abi.encodeWithSelector(Hooks.HookAddressNotValid.selector, address(key0.hooks))); - manager.initialize(key0, sqrtPriceX96, ZERO_BYTES); + manager.initialize(key0, sqrtPriceX96); } else if ((key0.fee != LPFeeLibrary.DYNAMIC_FEE_FLAG) && (key0.fee > 1000000)) { vm.expectRevert(abi.encodeWithSelector(LPFeeLibrary.LPFeeTooLarge.selector, key0.fee)); - manager.initialize(key0, sqrtPriceX96, ZERO_BYTES); + manager.initialize(key0, sqrtPriceX96); } else { int24 tick = TickMath.getTickAtSqrtPrice(sqrtPriceX96); vm.expectEmit(true, true, true, true); emit Initialize( key0.toId(), key0.currency0, key0.currency1, key0.fee, key0.tickSpacing, key0.hooks, sqrtPriceX96, tick ); - manager.initialize(key0, sqrtPriceX96, ZERO_BYTES); + manager.initialize(key0, sqrtPriceX96); (uint160 slot0SqrtPriceX96,, uint24 slot0ProtocolFee,) = manager.getSlot0(key0.toId()); assertEq(slot0SqrtPriceX96, sqrtPriceX96); @@ -112,7 +112,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { sqrtPriceX96, tick ); - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + manager.initialize(uninitializedKey, sqrtPriceX96); (uint160 slot0SqrtPriceX96, int24 slot0Tick, uint24 slot0ProtocolFee,) = manager.getSlot0(uninitializedKey.toId()); @@ -136,15 +136,15 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { uninitializedKey.hooks = IHooks(mockAddr); - int24 tick = manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + int24 tick = manager.initialize(uninitializedKey, sqrtPriceX96); (uint160 slot0SqrtPriceX96,,,) = manager.getSlot0(uninitializedKey.toId()); assertEq(slot0SqrtPriceX96, sqrtPriceX96, "sqrtPrice"); bytes32 beforeSelector = MockHooks.beforeInitialize.selector; - bytes memory beforeParams = abi.encode(address(this), uninitializedKey, sqrtPriceX96, ZERO_BYTES); + bytes memory beforeParams = abi.encode(address(this), uninitializedKey, sqrtPriceX96); bytes32 afterSelector = MockHooks.afterInitialize.selector; - bytes memory afterParams = abi.encode(address(this), uninitializedKey, sqrtPriceX96, tick, ZERO_BYTES); + bytes memory afterParams = abi.encode(address(this), uninitializedKey, sqrtPriceX96, tick); assertEq(MockContract(mockAddr).timesCalledSelector(beforeSelector), 1, "beforeSelector count"); assertTrue(MockContract(mockAddr).calledWithSelector(beforeSelector, beforeParams), "beforeSelector params"); @@ -172,7 +172,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { tick ); - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + manager.initialize(uninitializedKey, sqrtPriceX96); } function test_initialize_succeedsWithEmptyHooks(uint160 sqrtPriceX96) public { @@ -187,7 +187,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { uninitializedKey.hooks = mockHooks; - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + manager.initialize(uninitializedKey, sqrtPriceX96); (uint160 slot0SqrtPriceX96,,,) = manager.getSlot0(uninitializedKey.toId()); assertEq(slot0SqrtPriceX96, sqrtPriceX96); } @@ -206,7 +206,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { Currency.unwrap(currency0) ) ); - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + manager.initialize(uninitializedKey, sqrtPriceX96); } function test_initialize_revertsWithSameTokenCombo(uint160 sqrtPriceX96) public { @@ -223,7 +223,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { Currency.unwrap(currency0) ) ); - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + manager.initialize(uninitializedKey, sqrtPriceX96); } function test_initialize_fetchFeeWhenController(uint24 protocolFee) public { @@ -233,7 +233,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { uint16 fee0 = protocolFee.getZeroForOneFee(); uint16 fee1 = protocolFee.getOneForZeroFee(); - manager.initialize(uninitializedKey, SQRT_PRICE_1_1, ZERO_BYTES); + manager.initialize(uninitializedKey, SQRT_PRICE_1_1); (uint160 slot0SqrtPriceX96,, uint24 slot0ProtocolFee,) = manager.getSlot0(uninitializedKey.toId()); assertEq(slot0SqrtPriceX96, SQRT_PRICE_1_1); @@ -248,9 +248,9 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { // Assumptions tested in Pool.t.sol sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_PRICE, TickMath.MAX_SQRT_PRICE - 1)); - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + manager.initialize(uninitializedKey, sqrtPriceX96); vm.expectRevert(Pool.PoolAlreadyInitialized.selector); - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + manager.initialize(uninitializedKey, sqrtPriceX96); } function test_initialize_failsWithIncorrectSelectors() public { @@ -267,12 +267,12 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { // Fails at beforeInitialize hook. vm.expectRevert(Hooks.InvalidHookResponse.selector); - manager.initialize(uninitializedKey, SQRT_PRICE_1_1, ZERO_BYTES); + manager.initialize(uninitializedKey, SQRT_PRICE_1_1); // Fail at afterInitialize hook. mockHooks.setReturnValue(mockHooks.beforeInitialize.selector, mockHooks.beforeInitialize.selector); vm.expectRevert(Hooks.InvalidHookResponse.selector); - manager.initialize(uninitializedKey, SQRT_PRICE_1_1, ZERO_BYTES); + manager.initialize(uninitializedKey, SQRT_PRICE_1_1); } function test_initialize_succeedsWithCorrectSelectors() public { @@ -301,7 +301,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { tick ); - manager.initialize(uninitializedKey, SQRT_PRICE_1_1, ZERO_BYTES); + manager.initialize(uninitializedKey, SQRT_PRICE_1_1); } function test_initialize_failsIfTickSpaceTooLarge(uint160 sqrtPriceX96) public { @@ -311,7 +311,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { uninitializedKey.tickSpacing = TickMath.MAX_TICK_SPACING + 1; vm.expectRevert(abi.encodeWithSelector(IPoolManager.TickSpacingTooLarge.selector, uninitializedKey.tickSpacing)); - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + manager.initialize(uninitializedKey, sqrtPriceX96); } function test_initialize_failsIfTickSpaceZero(uint160 sqrtPriceX96) public { @@ -321,7 +321,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { uninitializedKey.tickSpacing = 0; vm.expectRevert(abi.encodeWithSelector(IPoolManager.TickSpacingTooSmall.selector, uninitializedKey.tickSpacing)); - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + manager.initialize(uninitializedKey, sqrtPriceX96); } function test_initialize_failsIfTickSpaceNeg(uint160 sqrtPriceX96) public { @@ -331,7 +331,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { uninitializedKey.tickSpacing = -1; vm.expectRevert(abi.encodeWithSelector(IPoolManager.TickSpacingTooSmall.selector, uninitializedKey.tickSpacing)); - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + manager.initialize(uninitializedKey, sqrtPriceX96); } function test_initialize_succeedsWithOutOfBoundsFeeController(uint160 sqrtPriceX96) public { @@ -353,7 +353,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { sqrtPriceX96, tick ); - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + manager.initialize(uninitializedKey, sqrtPriceX96); // protocol fees should default to 0 (,, uint24 slot0ProtocolFee,) = manager.getSlot0(uninitializedKey.toId()); assertEq(slot0ProtocolFee, 0); @@ -379,7 +379,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { sqrtPriceX96, tick ); - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + manager.initialize(uninitializedKey, sqrtPriceX96); // protocol fees should default to 0 (,, uint24 slot0ProtocolFee,) = manager.getSlot0(uninitializedKey.toId()); assertEq(slot0ProtocolFee, 0); @@ -405,7 +405,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { sqrtPriceX96, tick ); - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + manager.initialize(uninitializedKey, sqrtPriceX96); // protocol fees should default to 0 (,, uint24 slot0ProtocolFee,) = manager.getSlot0(uninitializedKey.toId()); assertEq(slot0ProtocolFee, 0); @@ -431,14 +431,14 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { sqrtPriceX96, tick ); - manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + 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, ZERO_BYTES); + manager.initialize(uninitializedKey, SQRT_PRICE_1_1); snapLastCall("initialize"); } } diff --git a/test/SkipCallsTestHook.t.sol b/test/SkipCallsTestHook.t.sol index fb07221aa..1e6bc584e 100644 --- a/test/SkipCallsTestHook.t.sol +++ b/test/SkipCallsTestHook.t.sol @@ -30,7 +30,7 @@ contract SkipCallsTest is Test, Deployers, GasSnapshot { assertEq(skipCallsTestHook.counter(), 0); - (key,) = initPool(currency0, currency1, IHooks(address(skipCallsTestHook)), 3000, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPool(currency0, currency1, IHooks(address(skipCallsTestHook)), 3000, SQRT_PRICE_1_1); } function approveAndAddLiquidity(SkipCallsTestHook skipCallsTestHook) private { diff --git a/test/Sync.t.sol b/test/Sync.t.sol index bc70386fb..74bf8e800 100644 --- a/test/Sync.t.sol +++ b/test/Sync.t.sol @@ -108,7 +108,7 @@ contract SyncTest is Test, Deployers, GasSnapshot { PoolKey memory key2 = PoolKey({currency0: cur0, currency1: cur1, fee: 3000, tickSpacing: 60, hooks: IHooks(address(0))}); - manager.initialize(key2, SQRT_PRICE_1_1, new bytes(0)); + manager.initialize(key2, SQRT_PRICE_1_1); modifyLiquidityRouter.modifyLiquidity(key2, IPoolManager.ModifyLiquidityParams(-60, 60, 100, 0), new bytes(0)); (uint256 balanceCurrency2) = currency2.balanceOf(address(manager)); diff --git a/test/libraries/Hooks.t.sol b/test/libraries/Hooks.t.sol index 610250ccb..eacc1ea6c 100644 --- a/test/libraries/Hooks.t.sol +++ b/test/libraries/Hooks.t.sol @@ -43,7 +43,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { } function test_initialize_succeedsWithHook() public { - manager.initialize(uninitializedKey, SQRT_PRICE_1_1, new bytes(123)); + manager.initialize(uninitializedKey, SQRT_PRICE_1_1); (uint160 sqrtPriceX96,,,) = manager.getSlot0(uninitializedKey.toId()); assertEq(sqrtPriceX96, SQRT_PRICE_1_1); @@ -54,13 +54,13 @@ contract HooksTest is Test, Deployers, GasSnapshot { function test_beforeInitialize_invalidReturn() public { mockHooks.setReturnValue(mockHooks.beforeInitialize.selector, bytes4(0xdeadbeef)); vm.expectRevert(Hooks.InvalidHookResponse.selector); - manager.initialize(uninitializedKey, SQRT_PRICE_1_1, ZERO_BYTES); + manager.initialize(uninitializedKey, SQRT_PRICE_1_1); } function test_afterInitialize_invalidReturn() public { mockHooks.setReturnValue(mockHooks.afterInitialize.selector, bytes4(0xdeadbeef)); vm.expectRevert(Hooks.InvalidHookResponse.selector); - manager.initialize(uninitializedKey, SQRT_PRICE_1_1, ZERO_BYTES); + manager.initialize(uninitializedKey, SQRT_PRICE_1_1); } function test_beforeAfterAddLiquidity_beforeAfterRemoveLiquidity_succeedsWithHook() public { @@ -1001,7 +1001,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { BaseTestHooks revertingHook = BaseTestHooks(beforeSwapFlag); PoolKey memory key = PoolKey(currency0, currency1, 0, 60, IHooks(revertingHook)); - manager.initialize(key, SQRT_PRICE_1_1, new bytes(0)); + manager.initialize(key, SQRT_PRICE_1_1); IPoolManager.SwapParams memory swapParams = IPoolManager.SwapParams({zeroForOne: true, amountSpecified: 100, sqrtPriceLimitX96: SQRT_PRICE_1_2}); @@ -1027,7 +1027,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { EmptyRevertContract revertingHook = EmptyRevertContract(beforeSwapFlag); PoolKey memory key = PoolKey(currency0, currency1, 0, 60, IHooks(address(revertingHook))); - manager.initialize(key, SQRT_PRICE_1_1, new bytes(0)); + manager.initialize(key, SQRT_PRICE_1_1); IPoolManager.SwapParams memory swapParams = IPoolManager.SwapParams({zeroForOne: true, amountSpecified: 100, sqrtPriceLimitX96: SQRT_PRICE_1_2}); diff --git a/test/libraries/StateLibrary.t.sol b/test/libraries/StateLibrary.t.sol index 8327b40f5..4f6294465 100644 --- a/test/libraries/StateLibrary.t.sol +++ b/test/libraries/StateLibrary.t.sol @@ -33,7 +33,7 @@ contract StateLibraryTest is Test, Deployers, Fuzzers, GasSnapshot { // Create the pool key = PoolKey(currency0, currency1, 3000, 60, IHooks(address(0x0))); poolId = key.toId(); - manager.initialize(key, SQRT_PRICE_1_1, ZERO_BYTES); + manager.initialize(key, SQRT_PRICE_1_1); } function test_getSlot0() public { diff --git a/test/utils/Deployers.sol b/test/utils/Deployers.sol index 2d7b6f410..9cf76b4a6 100644 --- a/test/utils/Deployers.sol +++ b/test/utils/Deployers.sol @@ -154,17 +154,13 @@ contract Deployers { } } - function initPool( - Currency _currency0, - Currency _currency1, - IHooks hooks, - uint24 fee, - uint160 sqrtPriceX96, - bytes memory initData - ) internal returns (PoolKey memory _key, PoolId id) { + function initPool(Currency _currency0, Currency _currency1, IHooks hooks, uint24 fee, uint160 sqrtPriceX96) + internal + returns (PoolKey memory _key, PoolId id) + { _key = PoolKey(_currency0, _currency1, fee, fee.isDynamicFee() ? int24(60) : int24(fee / 100 * 2), hooks); id = _key.toId(); - manager.initialize(_key, sqrtPriceX96, initData); + manager.initialize(_key, sqrtPriceX96); } function initPool( @@ -173,12 +169,11 @@ contract Deployers { IHooks hooks, uint24 fee, int24 tickSpacing, - uint160 sqrtPriceX96, - bytes memory initData + uint160 sqrtPriceX96 ) internal returns (PoolKey memory _key, PoolId id) { _key = PoolKey(_currency0, _currency1, fee, tickSpacing, hooks); id = _key.toId(); - manager.initialize(_key, sqrtPriceX96, initData); + manager.initialize(_key, sqrtPriceX96); } function initPoolAndAddLiquidity( @@ -186,10 +181,9 @@ contract Deployers { Currency _currency1, IHooks hooks, uint24 fee, - uint160 sqrtPriceX96, - bytes memory initData + uint160 sqrtPriceX96 ) internal returns (PoolKey memory _key, PoolId id) { - (_key, id) = initPool(_currency0, _currency1, hooks, fee, sqrtPriceX96, initData); + (_key, id) = initPool(_currency0, _currency1, hooks, fee, sqrtPriceX96); modifyLiquidityRouter.modifyLiquidity{value: msg.value}(_key, LIQUIDITY_PARAMS, ZERO_BYTES); } @@ -199,10 +193,9 @@ contract Deployers { IHooks hooks, uint24 fee, uint160 sqrtPriceX96, - bytes memory initData, uint256 msgValue ) internal returns (PoolKey memory _key, PoolId id) { - (_key, id) = initPool(_currency0, _currency1, hooks, fee, sqrtPriceX96, initData); + (_key, id) = initPool(_currency0, _currency1, hooks, fee, sqrtPriceX96); modifyLiquidityRouter.modifyLiquidity{value: msgValue}(_key, LIQUIDITY_PARAMS, ZERO_BYTES); } @@ -211,11 +204,10 @@ contract Deployers { deployFreshManagerAndRouters(); // sets the global currencies and key deployMintAndApprove2Currencies(); - (key,) = initPoolAndAddLiquidity(currency0, currency1, hooks, 3000, SQRT_PRICE_1_1, ZERO_BYTES); + (key,) = initPoolAndAddLiquidity(currency0, currency1, hooks, 3000, SQRT_PRICE_1_1); nestedActionRouter.executor().setKey(key); - (nativeKey,) = initPoolAndAddLiquidityETH( - CurrencyLibrary.ADDRESS_ZERO, currency1, hooks, 3000, SQRT_PRICE_1_1, ZERO_BYTES, 1 ether - ); + (nativeKey,) = + initPoolAndAddLiquidityETH(CurrencyLibrary.ADDRESS_ZERO, currency1, hooks, 3000, SQRT_PRICE_1_1, 1 ether); uninitializedKey = key; uninitializedNativeKey = nativeKey; uninitializedKey.fee = 100; From 3a41caa0ae661dc56eee319c10c94a6eba934e59 Mon Sep 17 00:00:00 2001 From: mr-uniswap <144828035+mr-uniswap@users.noreply.github.com> Date: Wed, 9 Oct 2024 18:31:16 -0400 Subject: [PATCH 3/9] Create CODEOWNERS (#897) --- .github/CODEOWNERS | 1 + 1 file changed, 1 insertion(+) create mode 100644 .github/CODEOWNERS 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 From 5412ddcbfeb4e32e854f9fc2e94829a004d1a352 Mon Sep 17 00:00:00 2001 From: mr-uniswap <144828035+mr-uniswap@users.noreply.github.com> Date: Wed, 9 Oct 2024 18:43:56 -0400 Subject: [PATCH 4/9] fix: disable template injection --- .github/workflows/coverage.yml | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 0bcf3c9de..c308cf5e2 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({ From 7189710dbf4b436f7edde2fd7a109ae61f552ad2 Mon Sep 17 00:00:00 2001 From: mr-uniswap <144828035+mr-uniswap@users.noreply.github.com> Date: Wed, 9 Oct 2024 18:57:30 -0400 Subject: [PATCH 5/9] lint (#898) --- .github/workflows/coverage.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index c308cf5e2..e5524a5f7 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -40,7 +40,7 @@ jobs: - name: Check coverage is updated uses: actions/github-script@v5 - env: + env: OUTPUT: ${{ steps.coverage.outputs.COVERAGE }} with: github-token: ${{ secrets.GITHUB_TOKEN }} @@ -61,7 +61,7 @@ jobs: - name: Comment on PR id: comment uses: actions/github-script@v5 - env: + env: OUTPUT: ${{ steps.coverage.outputs.COVERAGE }} with: github-token: ${{ secrets.GITHUB_TOKEN }} From f53af1bf17fd963d648443b65e34c9432bb8e5a1 Mon Sep 17 00:00:00 2001 From: mr-uniswap <144828035+mr-uniswap@users.noreply.github.com> Date: Thu, 10 Oct 2024 14:07:38 -0400 Subject: [PATCH 6/9] Use runner for forge tests (#900) * Use runner for forge tests Uses runner for forge tests * sets it for workflow dispatch --- .github/workflows/tests-merge.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 From fc5b4cf317f014ba96fa92f3ea964e03c58acbdd Mon Sep 17 00:00:00 2001 From: Alice <34962750+hensha256@users.noreply.github.com> Date: Thu, 10 Oct 2024 17:40:54 -0400 Subject: [PATCH 7/9] Remove pull of protocol fee (#887) * Remove pull of protocol fee * remove unused library * Typo and remove unused error --- ...o already existing position with salt.snap | 2 +- .forge-snapshots/addLiquidity CA fee.snap | 2 +- .../addLiquidity with empty hook.snap | 2 +- .../addLiquidity with native token.snap | 2 +- ...new liquidity to a position with salt.snap | 2 +- .forge-snapshots/donate gas with 1 token.snap | 2 +- .../donate gas with 2 tokens.snap | 2 +- .../erc20 collect protocol fees.snap | 2 +- .forge-snapshots/initialize.snap | 2 +- .../poolManager bytecode size.snap | 2 +- .forge-snapshots/removeLiquidity CA fee.snap | 2 +- .../removeLiquidity with empty hook.snap | 2 +- .../removeLiquidity with native token.snap | 2 +- ...dLiquidity second addition same range.snap | 2 +- .forge-snapshots/simple addLiquidity.snap | 2 +- ...emoveLiquidity some liquidity remains.snap | 2 +- .forge-snapshots/simple removeLiquidity.snap | 2 +- .forge-snapshots/simple swap with native.snap | 2 +- .forge-snapshots/simple swap.snap | 2 +- .../swap CA custom curve + swap noop.snap | 2 +- .../swap CA fee on unspecified.snap | 2 +- ...p against liquidity with native token.snap | 2 +- .forge-snapshots/swap against liquidity.snap | 2 +- .../swap burn 6909 for input.snap | 2 +- .../swap burn native 6909 for input.snap | 2 +- .../swap mint native output as 6909.snap | 2 +- .../swap mint output as 6909.snap | 2 +- ...wap skips hook call if hook is caller.snap | 2 +- .forge-snapshots/swap with hooks.snap | 2 +- .../swap with return dynamic fee.snap | 2 +- src/PoolManager.sol | 7 +- src/ProtocolFees.sol | 42 ------ src/interfaces/IProtocolFees.sol | 3 +- src/libraries/BipsLibrary.sol | 17 --- src/libraries/Hooks.sol | 4 +- src/libraries/Pool.sol | 11 +- src/test/ProtocolFeeControllerTest.sol | 39 ------ src/test/ProtocolFeesImplementation.sol | 4 - src/test/ProxyPoolManager.sol | 3 +- test/PoolManagerInitialize.t.sol | 121 ------------------ test/ProtocolFeesImplementation.t.sol | 47 +------ test/libraries/BipsLibrary.t.sol | 51 -------- test/libraries/Pool.t.sol | 26 ++-- test/utils/Deployers.sol | 16 +-- 44 files changed, 56 insertions(+), 395 deletions(-) delete mode 100644 src/libraries/BipsLibrary.sol delete mode 100644 test/libraries/BipsLibrary.t.sol 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 567cfb72b..342163227 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 @@ -144639 \ No newline at end of file +144663 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap index 13f0d90d7..d4dff6e54 100644 --- a/.forge-snapshots/addLiquidity CA fee.snap +++ b/.forge-snapshots/addLiquidity CA fee.snap @@ -1 +1 @@ -170935 \ No newline at end of file +170959 \ 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 b04e884a5..9d9c26d43 100644 --- a/.forge-snapshots/addLiquidity with empty hook.snap +++ b/.forge-snapshots/addLiquidity with empty hook.snap @@ -1 +1 @@ -274240 \ No newline at end of file +274264 \ 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 5c4d0bea7..a7684fdf5 100644 --- a/.forge-snapshots/addLiquidity with native token.snap +++ b/.forge-snapshots/addLiquidity with native token.snap @@ -1 +1 @@ -135129 \ No newline at end of file +135141 \ 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 521237ab9..3c4d3e51e 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 @@ -292831 \ No newline at end of file +292855 \ 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 a54ba22c4..cb54169dc 100644 --- a/.forge-snapshots/donate gas with 1 token.snap +++ b/.forge-snapshots/donate gas with 1 token.snap @@ -1 +1 @@ -106321 \ No newline at end of file +106345 \ 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 b5e007e06..0f05365bd 100644 --- a/.forge-snapshots/donate gas with 2 tokens.snap +++ b/.forge-snapshots/donate gas with 2 tokens.snap @@ -1 +1 @@ -145742 \ No newline at end of file +145766 \ 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 bb8641235..7606bacbf 100644 --- a/.forge-snapshots/erc20 collect protocol fees.snap +++ b/.forge-snapshots/erc20 collect protocol fees.snap @@ -1 +1 @@ -57442 \ No newline at end of file +57454 \ 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 2666adbeb..37be6af7a 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -24001 \ No newline at end of file +23621 \ 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 8d0e2523f..85250c467 100644 --- a/.forge-snapshots/simple addLiquidity second addition same range.snap +++ b/.forge-snapshots/simple addLiquidity second addition same range.snap @@ -1 +1 @@ -98838 \ No newline at end of file +98862 \ No newline at end of file diff --git a/.forge-snapshots/simple addLiquidity.snap b/.forge-snapshots/simple addLiquidity.snap index 2ac3df3f0..16a8472c7 100644 --- a/.forge-snapshots/simple addLiquidity.snap +++ b/.forge-snapshots/simple addLiquidity.snap @@ -1 +1 @@ -161383 \ No newline at end of file +161407 \ 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 98a88b74c..141489c72 100644 --- a/.forge-snapshots/simple swap.snap +++ b/.forge-snapshots/simple swap.snap @@ -1 +1 @@ -123323 \ No newline at end of file +123347 \ 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 c4cdede27..8ad9f5434 100644 --- a/.forge-snapshots/swap CA custom curve + swap noop.snap +++ b/.forge-snapshots/swap CA custom curve + swap noop.snap @@ -1 +1 @@ -124629 \ No newline at end of file +124653 \ 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 8f0ca3957..6360928ab 100644 --- a/.forge-snapshots/swap CA fee on unspecified.snap +++ b/.forge-snapshots/swap CA fee on unspecified.snap @@ -1 +1 @@ -154749 \ No newline at end of file +154773 \ 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 414122f63..0452eb72f 100644 --- a/.forge-snapshots/swap against liquidity.snap +++ b/.forge-snapshots/swap against liquidity.snap @@ -1 +1 @@ -116693 \ No newline at end of file +116717 \ 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 7cea3658b..63a0a75cf 100644 --- a/.forge-snapshots/swap mint native output as 6909.snap +++ b/.forge-snapshots/swap mint native output as 6909.snap @@ -1 +1 @@ -139735 \ No newline at end of file +139747 \ 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 f24b9c4e7..72228b87c 100644 --- a/.forge-snapshots/swap mint output as 6909.snap +++ b/.forge-snapshots/swap mint output as 6909.snap @@ -1 +1 @@ -155164 \ No newline at end of file +155188 \ 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 a98a52ade..f1e126934 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 @@ -206415 \ No newline at end of file +206403 \ No newline at end of file diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap index f6ae0682c..f36933737 100644 --- a/.forge-snapshots/swap with hooks.snap +++ b/.forge-snapshots/swap with hooks.snap @@ -1 +1 @@ -132321 \ No newline at end of file +132345 \ 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 09bc9151b..bcbae20f7 100644 --- a/.forge-snapshots/swap with return dynamic fee.snap +++ b/.forge-snapshots/swap with return dynamic fee.snap @@ -1 +1 @@ -145673 \ No newline at end of file +145661 \ No newline at end of file diff --git a/src/PoolManager.sol b/src/PoolManager.sol index f7d4b529f..92b8148ad 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 f89566872..ecea47484 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 @@ -66,42 +60,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 cfbf82154..0717bfc88 100644 --- a/src/interfaces/IProtocolFees.sol +++ b/src/interfaces/IProtocolFees.sol @@ -8,10 +8,9 @@ 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); + /// @notice Thrown when the contract is unlocked error ContractUnlocked(); 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 716d8b1bf..f5f7007ea 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; @@ -186,43 +180,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); From 07ea70ccdb820762fa71235bfadb9185b65ead6b Mon Sep 17 00:00:00 2001 From: mr-uniswap <144828035+mr-uniswap@users.noreply.github.com> Date: Fri, 11 Oct 2024 09:31:38 -0400 Subject: [PATCH 8/9] Delete .github/workflows/coverage.yml (#899) Deletes the coverage action --- .github/workflows/coverage.yml | 94 ---------------------------------- 1 file changed, 94 deletions(-) delete mode 100644 .github/workflows/coverage.yml diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml deleted file mode 100644 index e5524a5f7..000000000 --- a/.github/workflows/coverage.yml +++ /dev/null @@ -1,94 +0,0 @@ -name: code coverage - -on: - pull_request: - branches: - - main - -jobs: - comment-forge-coverage: - runs-on: ubuntu-latest - permissions: - contents: read - pull-requests: write - - steps: - - name: Checkout code - uses: actions/checkout@v3 - with: - token: ${{ secrets.GITHUB_TOKEN }} - - - name: Install foundry - uses: foundry-rs/foundry-toolchain@v1 - with: - version: nightly - - name: Run Forge build - run: | - forge --version - forge build --sizes --via-ir - id: build - - name: Run forge coverage - id: coverage - run: | - { - echo 'COVERAGE<> "$GITHUB_OUTPUT" - env: - FOUNDRY_RPC_URL: "${{ secrets.RPC_URL }}" - - - 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)) { - console.log("Nothing to check"); - return - } - const currentCoverage = fs.readFileSync(file, "utf8").trim(); - const newCoverage = (`${ OUTPUT }`).trim(); - if (newCoverage != currentCoverage) { - core.setFailed(`Code coverage not updated. Run : forge coverage | grep '^|' | grep -v 'test/' > coverage.txt`); - } - - - 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, - issue_number: context.issue.number, - }) - - const botComment = comments.find(comment => comment.user.id === 41898282) - - const commentBody = `Forge code coverage:\n${ OUTPUT }\n`; - - if (botComment) { - github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: botComment.id, - body: commentBody - }) - } else { - github.rest.issues.createComment({ - issue_number: context.issue.number, - owner: context.repo.owner, - repo: context.repo.repo, - body: commentBody - }); - } From 3afa83a0e3790edb0d6d9e2289555f8e1dce211e Mon Sep 17 00:00:00 2001 From: mr-uniswap <144828035+mr-uniswap@users.noreply.github.com> Date: Fri, 11 Oct 2024 16:28:33 -0400 Subject: [PATCH 9/9] Revert "Use runner for forge tests" (#901) Revert "Use runner for forge tests (#900)" This reverts commit f53af1bf17fd963d648443b65e34c9432bb8e5a1. --- .github/workflows/tests-merge.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/tests-merge.yml b/.github/workflows/tests-merge.yml index e1b11646d..13da03c2b 100644 --- a/.github/workflows/tests-merge.yml +++ b/.github/workflows/tests-merge.yml @@ -1,7 +1,6 @@ name: Tests on: - workflow_dispatch: push: branches: - main @@ -9,8 +8,7 @@ on: jobs: forge-tests: name: Forge Tests - runs-on: - group: forge-testing + runs-on: ubuntu-latest steps: - uses: actions/checkout@v3