From 753310189cb5836049efc1da8bc572265fdb879d Mon Sep 17 00:00:00 2001 From: Junion Date: Fri, 7 Jun 2024 13:48:48 -0400 Subject: [PATCH 01/17] add TakingFee hook --- contracts/hooks/examples/TakingFee.sol | 88 +++++++++++++++ test/TakingFee.t.sol | 101 ++++++++++++++++++ .../TakingFeeImplementation.sol | 16 +++ 3 files changed, 205 insertions(+) create mode 100644 contracts/hooks/examples/TakingFee.sol create mode 100644 test/TakingFee.t.sol create mode 100644 test/shared/implementation/TakingFeeImplementation.sol diff --git a/contracts/hooks/examples/TakingFee.sol b/contracts/hooks/examples/TakingFee.sol new file mode 100644 index 00000000..dbbe551a --- /dev/null +++ b/contracts/hooks/examples/TakingFee.sol @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import {BaseHook} from "../../BaseHook.sol"; +import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; +import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; +import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; +import {PoolId, PoolIdLibrary} from "@uniswap/v4-core/src/types/PoolId.sol"; +import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; +import {BeforeSwapDelta, BeforeSwapDeltaLibrary} from "@uniswap/v4-core/src/types/BeforeSwapDelta.sol"; +import {Currency} from "@uniswap/v4-core/src/types/Currency.sol"; +import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol"; +import {Owned} from "solmate/auth/Owned.sol"; + +contract TakingFee is BaseHook, Owned { + using PoolIdLibrary for PoolKey; + using SafeCast for uint256; + + uint128 private constant TOTAL_BIPS = 10000; + uint128 private constant MAX_BIPS = 100; + uint128 public swapFeeBips; + address public treasury = msg.sender; + + constructor( + IPoolManager _poolManager, + uint128 _swapFeeBips, + address _treasury + ) BaseHook(_poolManager) Owned(msg.sender) { + swapFeeBips = _swapFeeBips; + treasury = _treasury; + } + + function getHookPermissions() + public + pure + override + returns (Hooks.Permissions memory) + { + return + Hooks.Permissions({ + beforeInitialize: false, + afterInitialize: false, + beforeAddLiquidity: false, + afterAddLiquidity: false, + beforeRemoveLiquidity: false, + afterRemoveLiquidity: false, + beforeSwap: false, + afterSwap: true, + beforeDonate: false, + afterDonate: false, + beforeSwapReturnDelta: false, + afterSwapReturnDelta: true, + afterAddLiquidityReturnDelta: false, + afterRemoveLiquidityReturnDelta: false + }); + } + + function afterSwap( + address, + PoolKey calldata key, + IPoolManager.SwapParams calldata params, + BalanceDelta delta, + bytes calldata + ) external override returns (bytes4, int128) { + // fee will be in the unspecified token of the swap + bool specifiedTokenIs0 = (params.amountSpecified < 0 == + params.zeroForOne); + (Currency feeCurrency, int128 swapAmount) = (specifiedTokenIs0) + ? (key.currency1, delta.amount1()) + : (key.currency0, delta.amount0()); + // if fee is on output, get the absolute output amount + if (swapAmount < 0) swapAmount = -swapAmount; + + uint256 feeAmount = (uint128(swapAmount) * swapFeeBips) / TOTAL_BIPS; + poolManager.take(feeCurrency, treasury, feeAmount); + + return (BaseHook.afterSwap.selector, feeAmount.toInt128()); + } + + function setSwapFeeBips(uint128 _swapFeeBips) external onlyOwner { + require(_swapFeeBips <= MAX_BIPS); + swapFeeBips = _swapFeeBips; + } + + function setTreasury(address _treasury) external onlyOwner { + treasury = _treasury; + } +} diff --git a/test/TakingFee.t.sol b/test/TakingFee.t.sol new file mode 100644 index 00000000..18f48869 --- /dev/null +++ b/test/TakingFee.t.sol @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.19; + +import {Test} from "forge-std/Test.sol"; +import {GetSender} from "./shared/GetSender.sol"; +import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; +import {TakingFee} from "../contracts/hooks/examples/TakingFee.sol"; +import {TakingFeeImplementation} from "./shared/implementation/TakingFeeImplementation.sol"; +import {PoolManager} from "@uniswap/v4-core/src/PoolManager.sol"; +import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; +import {Deployers} from "@uniswap/v4-core/test/utils/Deployers.sol"; +import {TestERC20} from "@uniswap/v4-core/src/test/TestERC20.sol"; +import {CurrencyLibrary, Currency} from "@uniswap/v4-core/src/types/Currency.sol"; +import {PoolId, PoolIdLibrary} from "@uniswap/v4-core/src/types/PoolId.sol"; +import {TickMath} from "@uniswap/v4-core/src/libraries/TickMath.sol"; +import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; +import {HookEnabledSwapRouter} from "./utils/HookEnabledSwapRouter.sol"; +import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; +import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; + +contract TakingFeeTest is Test, Deployers { + using PoolIdLibrary for PoolKey; + using StateLibrary for IPoolManager; + + uint160 constant SQRT_RATIO_10_1 = 250541448375047931186413801569; + + address constant TREASURY = address(0x1234567890123456789012345678901234567890); + uint128 private constant TOTAL_BIPS = 10000; + + HookEnabledSwapRouter router; + TestERC20 token0; + TestERC20 token1; + TakingFee takingFee = TakingFee(address(uint160(Hooks.AFTER_SWAP_FLAG | Hooks.AFTER_SWAP_RETURNS_DELTA_FLAG))); + PoolId id; + + function setUp() public { + deployFreshManagerAndRouters(); + (currency0, currency1) = deployMintAndApprove2Currencies(); + + router = new HookEnabledSwapRouter(manager); + token0 = TestERC20(Currency.unwrap(currency0)); + token1 = TestERC20(Currency.unwrap(currency1)); + + vm.record(); + TakingFeeImplementation impl = new TakingFeeImplementation(manager, 25, TREASURY, takingFee); + (, bytes32[] memory writes) = vm.accesses(address(impl)); + vm.etch(address(takingFee), address(impl).code); + // for each storage key that was written during the hook implementation, copy the value over + unchecked { + for (uint256 i = 0; i < writes.length; i++) { + bytes32 slot = writes[i]; + vm.store(address(takingFee), slot, vm.load(address(impl), slot)); + } + } + + // key = PoolKey(currency0, currency1, 3000, 60, takingFee); + (key, id) = initPoolAndAddLiquidity(currency0, currency1, takingFee, 3000, SQRT_PRICE_1_1, ZERO_BYTES); + + token0.approve(address(takingFee), type(uint256).max); + token1.approve(address(takingFee), type(uint256).max); + token0.approve(address(router), type(uint256).max); + token1.approve(address(router), type(uint256).max); + } + + function testSwapHooks() public { + // rounding for tests + uint128 ROUND_FACTOR = 8; + + // positions were created in setup() + assertEq(currency0.balanceOf(TREASURY), 0); + assertEq(currency1.balanceOf(TREASURY), 0); + + // Perform a test swap // + bool zeroForOne = true; + int256 amountSpecified = -1e12; // negative number indicates exact input swap + BalanceDelta swapDelta = swap(key, zeroForOne, amountSpecified, ZERO_BYTES); + // ------------------- // + + uint128 output = uint128(swapDelta.amount1()); + assertFalse(output == 0); + + uint256 expectedFee = output * TOTAL_BIPS/(TOTAL_BIPS - takingFee.swapFeeBips()) - output; + + assertEq(currency0.balanceOf(TREASURY), 0); + assertEq(currency1.balanceOf(TREASURY) / ROUND_FACTOR, expectedFee / ROUND_FACTOR); + + // Perform a test swap // + bool zeroForOne2 = true; + int256 amountSpecified2 = 1e12; // positive number indicates exact output swap + BalanceDelta swapDelta2 = swap(key, zeroForOne2, amountSpecified2, ZERO_BYTES); + // ------------------- // + + uint128 input = uint128(-swapDelta2.amount0()); + assertFalse(input == 0); + + uint128 expectedFee2 = (input * takingFee.swapFeeBips()) / (TOTAL_BIPS + takingFee.swapFeeBips()); + + assertEq(currency0.balanceOf(TREASURY) / ROUND_FACTOR, expectedFee2 / ROUND_FACTOR); + assertEq(currency1.balanceOf(TREASURY) / ROUND_FACTOR, expectedFee / ROUND_FACTOR); + } +} diff --git a/test/shared/implementation/TakingFeeImplementation.sol b/test/shared/implementation/TakingFeeImplementation.sol new file mode 100644 index 00000000..e5e07237 --- /dev/null +++ b/test/shared/implementation/TakingFeeImplementation.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.19; + +import {BaseHook} from "../../../contracts/BaseHook.sol"; +import {TakingFee} from "../../../contracts/hooks/examples/TakingFee.sol"; +import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; +import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; + +contract TakingFeeImplementation is TakingFee { + constructor(IPoolManager _poolManager, uint128 _swapFeeBips, address _treasury, TakingFee addressToEtch) TakingFee(_poolManager, _swapFeeBips, _treasury) { + Hooks.validateHookPermissions(addressToEtch, getHookPermissions()); + } + + // make this a no-op in testing + function validateHookAddress(BaseHook _this) internal pure override {} +} From c2c5e9df05f22267c343067636568ae2b3169602 Mon Sep 17 00:00:00 2001 From: Junion Date: Fri, 7 Jun 2024 17:17:52 -0400 Subject: [PATCH 02/17] update implementation to use ERC6909 --- contracts/hooks/examples/TakingFee.sol | 94 ++++++++++--------- test/TakingFee.t.sol | 87 ++++++++++++----- .../TakingFeeImplementation.sol | 4 +- 3 files changed, 121 insertions(+), 64 deletions(-) diff --git a/contracts/hooks/examples/TakingFee.sol b/contracts/hooks/examples/TakingFee.sol index dbbe551a..d3294e6c 100644 --- a/contracts/hooks/examples/TakingFee.sol +++ b/contracts/hooks/examples/TakingFee.sol @@ -5,54 +5,46 @@ import {BaseHook} from "../../BaseHook.sol"; import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; -import {PoolId, PoolIdLibrary} from "@uniswap/v4-core/src/types/PoolId.sol"; import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; import {BeforeSwapDelta, BeforeSwapDeltaLibrary} from "@uniswap/v4-core/src/types/BeforeSwapDelta.sol"; -import {Currency} from "@uniswap/v4-core/src/types/Currency.sol"; +import {Currency, CurrencyLibrary} from "@uniswap/v4-core/src/types/Currency.sol"; import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol"; import {Owned} from "solmate/auth/Owned.sol"; +import {IUnlockCallback} from "@uniswap/v4-core/src/interfaces/callback/IUnlockCallback.sol"; -contract TakingFee is BaseHook, Owned { - using PoolIdLibrary for PoolKey; +contract TakingFee is BaseHook, IUnlockCallback, Owned { using SafeCast for uint256; uint128 private constant TOTAL_BIPS = 10000; uint128 private constant MAX_BIPS = 100; uint128 public swapFeeBips; - address public treasury = msg.sender; - constructor( - IPoolManager _poolManager, - uint128 _swapFeeBips, - address _treasury - ) BaseHook(_poolManager) Owned(msg.sender) { + struct CallbackData { + address to; + Currency[] currencies; + } + + constructor(IPoolManager _poolManager, uint128 _swapFeeBips) BaseHook(_poolManager) Owned(msg.sender) { swapFeeBips = _swapFeeBips; - treasury = _treasury; } - function getHookPermissions() - public - pure - override - returns (Hooks.Permissions memory) - { - return - Hooks.Permissions({ - beforeInitialize: false, - afterInitialize: false, - beforeAddLiquidity: false, - afterAddLiquidity: false, - beforeRemoveLiquidity: false, - afterRemoveLiquidity: false, - beforeSwap: false, - afterSwap: true, - beforeDonate: false, - afterDonate: false, - beforeSwapReturnDelta: false, - afterSwapReturnDelta: true, - afterAddLiquidityReturnDelta: false, - afterRemoveLiquidityReturnDelta: false - }); + function getHookPermissions() public pure override returns (Hooks.Permissions memory) { + return Hooks.Permissions({ + beforeInitialize: false, + afterInitialize: false, + beforeAddLiquidity: false, + afterAddLiquidity: false, + beforeRemoveLiquidity: false, + afterRemoveLiquidity: false, + beforeSwap: false, + afterSwap: true, + beforeDonate: false, + afterDonate: false, + beforeSwapReturnDelta: false, + afterSwapReturnDelta: true, + afterAddLiquidityReturnDelta: false, + afterRemoveLiquidityReturnDelta: false + }); } function afterSwap( @@ -63,16 +55,15 @@ contract TakingFee is BaseHook, Owned { bytes calldata ) external override returns (bytes4, int128) { // fee will be in the unspecified token of the swap - bool specifiedTokenIs0 = (params.amountSpecified < 0 == - params.zeroForOne); - (Currency feeCurrency, int128 swapAmount) = (specifiedTokenIs0) - ? (key.currency1, delta.amount1()) - : (key.currency0, delta.amount0()); + bool specifiedTokenIs0 = (params.amountSpecified < 0 == params.zeroForOne); + (Currency feeCurrency, int128 swapAmount) = + (specifiedTokenIs0) ? (key.currency1, delta.amount1()) : (key.currency0, delta.amount0()); // if fee is on output, get the absolute output amount if (swapAmount < 0) swapAmount = -swapAmount; uint256 feeAmount = (uint128(swapAmount) * swapFeeBips) / TOTAL_BIPS; - poolManager.take(feeCurrency, treasury, feeAmount); + // mint ERC6909 instead of take to avoid edge case where PM doesn't have enough balance + poolManager.mint(address(this), CurrencyLibrary.toId(feeCurrency), feeAmount); return (BaseHook.afterSwap.selector, feeAmount.toInt128()); } @@ -82,7 +73,26 @@ contract TakingFee is BaseHook, Owned { swapFeeBips = _swapFeeBips; } - function setTreasury(address _treasury) external onlyOwner { - treasury = _treasury; + function withdraw(address to, Currency[] calldata currencies) external onlyOwner { + poolManager.unlock(abi.encode(CallbackData(to, currencies))); + } + + function unlockCallback(bytes calldata rawData) + external + override(IUnlockCallback, BaseHook) + poolManagerOnly + returns (bytes memory) + { + CallbackData memory data = abi.decode(rawData, (CallbackData)); + uint256 length = data.currencies.length; + for (uint256 i = 0; i < length;) { + uint256 amount = poolManager.balanceOf(address(this), CurrencyLibrary.toId(data.currencies[i])); + poolManager.burn(address(this), CurrencyLibrary.toId(data.currencies[i]), amount); + poolManager.take(data.currencies[i], data.to, amount); + unchecked { + i++; + } + } + return ""; } } diff --git a/test/TakingFee.t.sol b/test/TakingFee.t.sol index 18f48869..7a1edd50 100644 --- a/test/TakingFee.t.sol +++ b/test/TakingFee.t.sol @@ -2,7 +2,6 @@ pragma solidity ^0.8.19; import {Test} from "forge-std/Test.sol"; -import {GetSender} from "./shared/GetSender.sol"; import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; import {TakingFee} from "../contracts/hooks/examples/TakingFee.sol"; import {TakingFeeImplementation} from "./shared/implementation/TakingFeeImplementation.sol"; @@ -12,7 +11,6 @@ import {Deployers} from "@uniswap/v4-core/test/utils/Deployers.sol"; import {TestERC20} from "@uniswap/v4-core/src/test/TestERC20.sol"; import {CurrencyLibrary, Currency} from "@uniswap/v4-core/src/types/Currency.sol"; import {PoolId, PoolIdLibrary} from "@uniswap/v4-core/src/types/PoolId.sol"; -import {TickMath} from "@uniswap/v4-core/src/libraries/TickMath.sol"; import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; import {HookEnabledSwapRouter} from "./utils/HookEnabledSwapRouter.sol"; import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; @@ -27,6 +25,9 @@ contract TakingFeeTest is Test, Deployers { address constant TREASURY = address(0x1234567890123456789012345678901234567890); uint128 private constant TOTAL_BIPS = 10000; + // rounding for tests to avoid floating point errors + uint128 R = 10; + HookEnabledSwapRouter router; TestERC20 token0; TestERC20 token1; @@ -42,7 +43,7 @@ contract TakingFeeTest is Test, Deployers { token1 = TestERC20(Currency.unwrap(currency1)); vm.record(); - TakingFeeImplementation impl = new TakingFeeImplementation(manager, 25, TREASURY, takingFee); + TakingFeeImplementation impl = new TakingFeeImplementation(manager, 25, takingFee); (, bytes32[] memory writes) = vm.accesses(address(impl)); vm.etch(address(takingFee), address(impl).code); // for each storage key that was written during the hook implementation, copy the value over @@ -63,39 +64,83 @@ contract TakingFeeTest is Test, Deployers { } function testSwapHooks() public { - // rounding for tests - uint128 ROUND_FACTOR = 8; - - // positions were created in setup() assertEq(currency0.balanceOf(TREASURY), 0); assertEq(currency1.balanceOf(TREASURY), 0); - // Perform a test swap // + // Swap exact token0 for token1 // bool zeroForOne = true; - int256 amountSpecified = -1e12; // negative number indicates exact input swap + int256 amountSpecified = -1e12; BalanceDelta swapDelta = swap(key, zeroForOne, amountSpecified, ZERO_BYTES); - // ------------------- // + // ---------------------------- // uint128 output = uint128(swapDelta.amount1()); - assertFalse(output == 0); + assertTrue(output > 0); - uint256 expectedFee = output * TOTAL_BIPS/(TOTAL_BIPS - takingFee.swapFeeBips()) - output; + uint256 expectedFee = output * TOTAL_BIPS / (TOTAL_BIPS - takingFee.swapFeeBips()) - output; - assertEq(currency0.balanceOf(TREASURY), 0); - assertEq(currency1.balanceOf(TREASURY) / ROUND_FACTOR, expectedFee / ROUND_FACTOR); - - // Perform a test swap // + assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); + + // Swap token0 for exact token1 // bool zeroForOne2 = true; int256 amountSpecified2 = 1e12; // positive number indicates exact output swap BalanceDelta swapDelta2 = swap(key, zeroForOne2, amountSpecified2, ZERO_BYTES); - // ------------------- // - + // ---------------------------- // + uint128 input = uint128(-swapDelta2.amount0()); - assertFalse(input == 0); + assertTrue(output > 0); + + uint128 expectedFee2 = (input * takingFee.swapFeeBips()) / (TOTAL_BIPS + takingFee.swapFeeBips()); + + assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency0)) / R, expectedFee2 / R); + assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); + + // test withdrawing tokens // + Currency[] memory currencies = new Currency[](2); + currencies[0] = key.currency0; + currencies[1] = key.currency1; + takingFee.withdraw(TREASURY, currencies); + assertEq(manager.balanceOf(address(this), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(this), CurrencyLibrary.toId(key.currency1)), 0); + assertEq(currency0.balanceOf(TREASURY) / R, expectedFee2 / R); + assertEq(currency1.balanceOf(TREASURY) / R, expectedFee / R); + } + + function testEdgeCase() public { + // Swap exact token0 for token1 // + bool zeroForOne = true; + int256 amountSpecified = -1e18; + BalanceDelta swapDelta = swap(key, zeroForOne, amountSpecified, ZERO_BYTES); + // ---------------------------- // + + uint128 output = uint128(swapDelta.amount1()); + assertTrue(output > 0); + + uint256 expectedFee = output * TOTAL_BIPS / (TOTAL_BIPS - takingFee.swapFeeBips()) - output; + + assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); + + // Swap token1 for exact token0 // + bool zeroForOne2 = false; + int256 amountSpecified2 = 1e18; // positive number indicates exact output swap + BalanceDelta swapDelta2 = swap(key, zeroForOne2, amountSpecified2, ZERO_BYTES); + // ---------------------------- // + + uint128 input = uint128(-swapDelta2.amount1()); + assertTrue(output > 0); uint128 expectedFee2 = (input * takingFee.swapFeeBips()) / (TOTAL_BIPS + takingFee.swapFeeBips()); - assertEq(currency0.balanceOf(TREASURY) / ROUND_FACTOR, expectedFee2 / ROUND_FACTOR); - assertEq(currency1.balanceOf(TREASURY) / ROUND_FACTOR, expectedFee / ROUND_FACTOR); + assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency1)) / R, (expectedFee + expectedFee2) / R); + + // test withdrawing tokens // + Currency[] memory currencies = new Currency[](2); + currencies[0] = key.currency0; + currencies[1] = key.currency1; + takingFee.withdraw(TREASURY, currencies); + assertEq(currency0.balanceOf(TREASURY) / R, 0); + assertEq(currency1.balanceOf(TREASURY) / R, (expectedFee + expectedFee2) / R); } } diff --git a/test/shared/implementation/TakingFeeImplementation.sol b/test/shared/implementation/TakingFeeImplementation.sol index e5e07237..8c1a0c11 100644 --- a/test/shared/implementation/TakingFeeImplementation.sol +++ b/test/shared/implementation/TakingFeeImplementation.sol @@ -7,7 +7,9 @@ import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; contract TakingFeeImplementation is TakingFee { - constructor(IPoolManager _poolManager, uint128 _swapFeeBips, address _treasury, TakingFee addressToEtch) TakingFee(_poolManager, _swapFeeBips, _treasury) { + constructor(IPoolManager _poolManager, uint128 _swapFeeBips, TakingFee addressToEtch) + TakingFee(_poolManager, _swapFeeBips) + { Hooks.validateHookPermissions(addressToEtch, getHookPermissions()); } From 2db74673c47587b032ad1f93a31ff5ebd09b830b Mon Sep 17 00:00:00 2001 From: Junion Date: Fri, 7 Jun 2024 17:24:45 -0400 Subject: [PATCH 03/17] forge fmt --- test/TakingFee.t.sol | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/TakingFee.t.sol b/test/TakingFee.t.sol index 7a1edd50..07af4faf 100644 --- a/test/TakingFee.t.sol +++ b/test/TakingFee.t.sol @@ -107,6 +107,7 @@ contract TakingFeeTest is Test, Deployers { } function testEdgeCase() public { + // first, deplete the pool of token1 // Swap exact token0 for token1 // bool zeroForOne = true; int256 amountSpecified = -1e18; @@ -133,7 +134,10 @@ contract TakingFeeTest is Test, Deployers { uint128 expectedFee2 = (input * takingFee.swapFeeBips()) / (TOTAL_BIPS + takingFee.swapFeeBips()); assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency0)), 0); - assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency1)) / R, (expectedFee + expectedFee2) / R); + assertEq( + manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency1)) / R, + (expectedFee + expectedFee2) / R + ); // test withdrawing tokens // Currency[] memory currencies = new Currency[](2); From 59da5a9bc4be5ce4f877c1b94b5d26ea205ed8f7 Mon Sep 17 00:00:00 2001 From: Junion Date: Thu, 13 Jun 2024 10:20:12 -0400 Subject: [PATCH 04/17] resolve nit --- contracts/hooks/examples/TakingFee.sol | 6 +-- test/TakingFee.t.sol | 50 +++++++++---------- .../TakingFeeImplementation.sol | 8 +-- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/contracts/hooks/examples/TakingFee.sol b/contracts/hooks/examples/TakingFee.sol index d3294e6c..07b04777 100644 --- a/contracts/hooks/examples/TakingFee.sol +++ b/contracts/hooks/examples/TakingFee.sol @@ -12,7 +12,7 @@ import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol"; import {Owned} from "solmate/auth/Owned.sol"; import {IUnlockCallback} from "@uniswap/v4-core/src/interfaces/callback/IUnlockCallback.sol"; -contract TakingFee is BaseHook, IUnlockCallback, Owned { +contract FeeTaking is BaseHook, IUnlockCallback, Owned { using SafeCast for uint256; uint128 private constant TOTAL_BIPS = 10000; @@ -55,9 +55,9 @@ contract TakingFee is BaseHook, IUnlockCallback, Owned { bytes calldata ) external override returns (bytes4, int128) { // fee will be in the unspecified token of the swap - bool specifiedTokenIs0 = (params.amountSpecified < 0 == params.zeroForOne); + bool currency0Specified = (params.amountSpecified < 0 == params.zeroForOne); (Currency feeCurrency, int128 swapAmount) = - (specifiedTokenIs0) ? (key.currency1, delta.amount1()) : (key.currency0, delta.amount0()); + (currency0Specified) ? (key.currency1, delta.amount1()) : (key.currency0, delta.amount0()); // if fee is on output, get the absolute output amount if (swapAmount < 0) swapAmount = -swapAmount; diff --git a/test/TakingFee.t.sol b/test/TakingFee.t.sol index 07af4faf..fb674031 100644 --- a/test/TakingFee.t.sol +++ b/test/TakingFee.t.sol @@ -3,8 +3,8 @@ pragma solidity ^0.8.19; import {Test} from "forge-std/Test.sol"; import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; -import {TakingFee} from "../contracts/hooks/examples/TakingFee.sol"; -import {TakingFeeImplementation} from "./shared/implementation/TakingFeeImplementation.sol"; +import {FeeTaking} from "../contracts/hooks/examples/FeeTaking.sol"; +import {FeeTakingImplementation} from "./shared/implementation/FeeTakingImplementation.sol"; import {PoolManager} from "@uniswap/v4-core/src/PoolManager.sol"; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; import {Deployers} from "@uniswap/v4-core/test/utils/Deployers.sol"; @@ -16,7 +16,7 @@ import {HookEnabledSwapRouter} from "./utils/HookEnabledSwapRouter.sol"; import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; -contract TakingFeeTest is Test, Deployers { +contract FeeTakingTest is Test, Deployers { using PoolIdLibrary for PoolKey; using StateLibrary for IPoolManager; @@ -31,7 +31,7 @@ contract TakingFeeTest is Test, Deployers { HookEnabledSwapRouter router; TestERC20 token0; TestERC20 token1; - TakingFee takingFee = TakingFee(address(uint160(Hooks.AFTER_SWAP_FLAG | Hooks.AFTER_SWAP_RETURNS_DELTA_FLAG))); + FeeTaking FeeTaking = FeeTaking(address(uint160(Hooks.AFTER_SWAP_FLAG | Hooks.AFTER_SWAP_RETURNS_DELTA_FLAG))); PoolId id; function setUp() public { @@ -43,22 +43,22 @@ contract TakingFeeTest is Test, Deployers { token1 = TestERC20(Currency.unwrap(currency1)); vm.record(); - TakingFeeImplementation impl = new TakingFeeImplementation(manager, 25, takingFee); + FeeTakingImplementation impl = new FeeTakingImplementation(manager, 25, FeeTaking); (, bytes32[] memory writes) = vm.accesses(address(impl)); - vm.etch(address(takingFee), address(impl).code); + vm.etch(address(FeeTaking), address(impl).code); // for each storage key that was written during the hook implementation, copy the value over unchecked { for (uint256 i = 0; i < writes.length; i++) { bytes32 slot = writes[i]; - vm.store(address(takingFee), slot, vm.load(address(impl), slot)); + vm.store(address(FeeTaking), slot, vm.load(address(impl), slot)); } } - // key = PoolKey(currency0, currency1, 3000, 60, takingFee); - (key, id) = initPoolAndAddLiquidity(currency0, currency1, takingFee, 3000, SQRT_PRICE_1_1, ZERO_BYTES); + // key = PoolKey(currency0, currency1, 3000, 60, FeeTaking); + (key, id) = initPoolAndAddLiquidity(currency0, currency1, FeeTaking, 3000, SQRT_PRICE_1_1, ZERO_BYTES); - token0.approve(address(takingFee), type(uint256).max); - token1.approve(address(takingFee), type(uint256).max); + token0.approve(address(FeeTaking), type(uint256).max); + token1.approve(address(FeeTaking), type(uint256).max); token0.approve(address(router), type(uint256).max); token1.approve(address(router), type(uint256).max); } @@ -76,10 +76,10 @@ contract TakingFeeTest is Test, Deployers { uint128 output = uint128(swapDelta.amount1()); assertTrue(output > 0); - uint256 expectedFee = output * TOTAL_BIPS / (TOTAL_BIPS - takingFee.swapFeeBips()) - output; + uint256 expectedFee = output * TOTAL_BIPS / (TOTAL_BIPS - FeeTaking.swapFeeBips()) - output; - assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency0)), 0); - assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); + assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); // Swap token0 for exact token1 // bool zeroForOne2 = true; @@ -90,16 +90,16 @@ contract TakingFeeTest is Test, Deployers { uint128 input = uint128(-swapDelta2.amount0()); assertTrue(output > 0); - uint128 expectedFee2 = (input * takingFee.swapFeeBips()) / (TOTAL_BIPS + takingFee.swapFeeBips()); + uint128 expectedFee2 = (input * FeeTaking.swapFeeBips()) / (TOTAL_BIPS + FeeTaking.swapFeeBips()); - assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency0)) / R, expectedFee2 / R); - assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); + assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency0)) / R, expectedFee2 / R); + assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); // test withdrawing tokens // Currency[] memory currencies = new Currency[](2); currencies[0] = key.currency0; currencies[1] = key.currency1; - takingFee.withdraw(TREASURY, currencies); + FeeTaking.withdraw(TREASURY, currencies); assertEq(manager.balanceOf(address(this), CurrencyLibrary.toId(key.currency0)), 0); assertEq(manager.balanceOf(address(this), CurrencyLibrary.toId(key.currency1)), 0); assertEq(currency0.balanceOf(TREASURY) / R, expectedFee2 / R); @@ -117,10 +117,10 @@ contract TakingFeeTest is Test, Deployers { uint128 output = uint128(swapDelta.amount1()); assertTrue(output > 0); - uint256 expectedFee = output * TOTAL_BIPS / (TOTAL_BIPS - takingFee.swapFeeBips()) - output; + uint256 expectedFee = output * TOTAL_BIPS / (TOTAL_BIPS - FeeTaking.swapFeeBips()) - output; - assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency0)), 0); - assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); + assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); // Swap token1 for exact token0 // bool zeroForOne2 = false; @@ -131,11 +131,11 @@ contract TakingFeeTest is Test, Deployers { uint128 input = uint128(-swapDelta2.amount1()); assertTrue(output > 0); - uint128 expectedFee2 = (input * takingFee.swapFeeBips()) / (TOTAL_BIPS + takingFee.swapFeeBips()); + uint128 expectedFee2 = (input * FeeTaking.swapFeeBips()) / (TOTAL_BIPS + FeeTaking.swapFeeBips()); - assertEq(manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency0)), 0); assertEq( - manager.balanceOf(address(takingFee), CurrencyLibrary.toId(key.currency1)) / R, + manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency1)) / R, (expectedFee + expectedFee2) / R ); @@ -143,7 +143,7 @@ contract TakingFeeTest is Test, Deployers { Currency[] memory currencies = new Currency[](2); currencies[0] = key.currency0; currencies[1] = key.currency1; - takingFee.withdraw(TREASURY, currencies); + FeeTaking.withdraw(TREASURY, currencies); assertEq(currency0.balanceOf(TREASURY) / R, 0); assertEq(currency1.balanceOf(TREASURY) / R, (expectedFee + expectedFee2) / R); } diff --git a/test/shared/implementation/TakingFeeImplementation.sol b/test/shared/implementation/TakingFeeImplementation.sol index 8c1a0c11..1d3bc051 100644 --- a/test/shared/implementation/TakingFeeImplementation.sol +++ b/test/shared/implementation/TakingFeeImplementation.sol @@ -2,13 +2,13 @@ pragma solidity ^0.8.19; import {BaseHook} from "../../../contracts/BaseHook.sol"; -import {TakingFee} from "../../../contracts/hooks/examples/TakingFee.sol"; +import {FeeTaking} from "../../../contracts/hooks/examples/FeeTaking.sol"; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; -contract TakingFeeImplementation is TakingFee { - constructor(IPoolManager _poolManager, uint128 _swapFeeBips, TakingFee addressToEtch) - TakingFee(_poolManager, _swapFeeBips) +contract FeeTakingImplementation is FeeTaking { + constructor(IPoolManager _poolManager, uint128 _swapFeeBips, FeeTaking addressToEtch) + FeeTaking(_poolManager, _swapFeeBips) { Hooks.validateHookPermissions(addressToEtch, getHookPermissions()); } From 4628fe1a213dc622b55ec8bfdc710a0d3f1b4429 Mon Sep 17 00:00:00 2001 From: Junion Date: Thu, 13 Jun 2024 10:21:41 -0400 Subject: [PATCH 05/17] file name change --- contracts/hooks/examples/{TakingFee.sol => FeeTaking.sol} | 0 test/{TakingFee.t.sol => FeeTaking.t.sol} | 0 .../{TakingFeeImplementation.sol => FeeTakingImplementation.sol} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename contracts/hooks/examples/{TakingFee.sol => FeeTaking.sol} (100%) rename test/{TakingFee.t.sol => FeeTaking.t.sol} (100%) rename test/shared/implementation/{TakingFeeImplementation.sol => FeeTakingImplementation.sol} (100%) diff --git a/contracts/hooks/examples/TakingFee.sol b/contracts/hooks/examples/FeeTaking.sol similarity index 100% rename from contracts/hooks/examples/TakingFee.sol rename to contracts/hooks/examples/FeeTaking.sol diff --git a/test/TakingFee.t.sol b/test/FeeTaking.t.sol similarity index 100% rename from test/TakingFee.t.sol rename to test/FeeTaking.t.sol diff --git a/test/shared/implementation/TakingFeeImplementation.sol b/test/shared/implementation/FeeTakingImplementation.sol similarity index 100% rename from test/shared/implementation/TakingFeeImplementation.sol rename to test/shared/implementation/FeeTakingImplementation.sol From da0ce0825d668d82f92d026f51af70daa3c8ddd3 Mon Sep 17 00:00:00 2001 From: Junion Date: Thu, 13 Jun 2024 11:38:07 -0400 Subject: [PATCH 06/17] fix bug --- test/FeeTaking.t.sol | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/test/FeeTaking.t.sol b/test/FeeTaking.t.sol index fb674031..028c7ebe 100644 --- a/test/FeeTaking.t.sol +++ b/test/FeeTaking.t.sol @@ -31,7 +31,7 @@ contract FeeTakingTest is Test, Deployers { HookEnabledSwapRouter router; TestERC20 token0; TestERC20 token1; - FeeTaking FeeTaking = FeeTaking(address(uint160(Hooks.AFTER_SWAP_FLAG | Hooks.AFTER_SWAP_RETURNS_DELTA_FLAG))); + FeeTaking feeTaking = FeeTaking(address(uint160(Hooks.AFTER_SWAP_FLAG | Hooks.AFTER_SWAP_RETURNS_DELTA_FLAG))); PoolId id; function setUp() public { @@ -43,22 +43,22 @@ contract FeeTakingTest is Test, Deployers { token1 = TestERC20(Currency.unwrap(currency1)); vm.record(); - FeeTakingImplementation impl = new FeeTakingImplementation(manager, 25, FeeTaking); + FeeTakingImplementation impl = new FeeTakingImplementation(manager, 25, feeTaking); (, bytes32[] memory writes) = vm.accesses(address(impl)); - vm.etch(address(FeeTaking), address(impl).code); + vm.etch(address(feeTaking), address(impl).code); // for each storage key that was written during the hook implementation, copy the value over unchecked { for (uint256 i = 0; i < writes.length; i++) { bytes32 slot = writes[i]; - vm.store(address(FeeTaking), slot, vm.load(address(impl), slot)); + vm.store(address(feeTaking), slot, vm.load(address(impl), slot)); } } - // key = PoolKey(currency0, currency1, 3000, 60, FeeTaking); - (key, id) = initPoolAndAddLiquidity(currency0, currency1, FeeTaking, 3000, SQRT_PRICE_1_1, ZERO_BYTES); + // key = PoolKey(currency0, currency1, 3000, 60, feeTaking); + (key, id) = initPoolAndAddLiquidity(currency0, currency1, feeTaking, 3000, SQRT_PRICE_1_1, ZERO_BYTES); - token0.approve(address(FeeTaking), type(uint256).max); - token1.approve(address(FeeTaking), type(uint256).max); + token0.approve(address(feeTaking), type(uint256).max); + token1.approve(address(feeTaking), type(uint256).max); token0.approve(address(router), type(uint256).max); token1.approve(address(router), type(uint256).max); } @@ -76,10 +76,10 @@ contract FeeTakingTest is Test, Deployers { uint128 output = uint128(swapDelta.amount1()); assertTrue(output > 0); - uint256 expectedFee = output * TOTAL_BIPS / (TOTAL_BIPS - FeeTaking.swapFeeBips()) - output; + uint256 expectedFee = output * TOTAL_BIPS / (TOTAL_BIPS - feeTaking.swapFeeBips()) - output; - assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency0)), 0); - assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); // Swap token0 for exact token1 // bool zeroForOne2 = true; @@ -90,16 +90,16 @@ contract FeeTakingTest is Test, Deployers { uint128 input = uint128(-swapDelta2.amount0()); assertTrue(output > 0); - uint128 expectedFee2 = (input * FeeTaking.swapFeeBips()) / (TOTAL_BIPS + FeeTaking.swapFeeBips()); + uint128 expectedFee2 = (input * feeTaking.swapFeeBips()) / (TOTAL_BIPS + feeTaking.swapFeeBips()); - assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency0)) / R, expectedFee2 / R); - assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)) / R, expectedFee2 / R); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); // test withdrawing tokens // Currency[] memory currencies = new Currency[](2); currencies[0] = key.currency0; currencies[1] = key.currency1; - FeeTaking.withdraw(TREASURY, currencies); + feeTaking.withdraw(TREASURY, currencies); assertEq(manager.balanceOf(address(this), CurrencyLibrary.toId(key.currency0)), 0); assertEq(manager.balanceOf(address(this), CurrencyLibrary.toId(key.currency1)), 0); assertEq(currency0.balanceOf(TREASURY) / R, expectedFee2 / R); @@ -117,10 +117,10 @@ contract FeeTakingTest is Test, Deployers { uint128 output = uint128(swapDelta.amount1()); assertTrue(output > 0); - uint256 expectedFee = output * TOTAL_BIPS / (TOTAL_BIPS - FeeTaking.swapFeeBips()) - output; + uint256 expectedFee = output * TOTAL_BIPS / (TOTAL_BIPS - feeTaking.swapFeeBips()) - output; - assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency0)), 0); - assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); // Swap token1 for exact token0 // bool zeroForOne2 = false; @@ -131,11 +131,11 @@ contract FeeTakingTest is Test, Deployers { uint128 input = uint128(-swapDelta2.amount1()); assertTrue(output > 0); - uint128 expectedFee2 = (input * FeeTaking.swapFeeBips()) / (TOTAL_BIPS + FeeTaking.swapFeeBips()); + uint128 expectedFee2 = (input * feeTaking.swapFeeBips()) / (TOTAL_BIPS + feeTaking.swapFeeBips()); - assertEq(manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); assertEq( - manager.balanceOf(address(FeeTaking), CurrencyLibrary.toId(key.currency1)) / R, + manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)) / R, (expectedFee + expectedFee2) / R ); @@ -143,7 +143,7 @@ contract FeeTakingTest is Test, Deployers { Currency[] memory currencies = new Currency[](2); currencies[0] = key.currency0; currencies[1] = key.currency1; - FeeTaking.withdraw(TREASURY, currencies); + feeTaking.withdraw(TREASURY, currencies); assertEq(currency0.balanceOf(TREASURY) / R, 0); assertEq(currency1.balanceOf(TREASURY) / R, (expectedFee + expectedFee2) / R); } From 0a61b061cdc20fce7e9ab854b9ffc427966be250 Mon Sep 17 00:00:00 2001 From: Junion Date: Thu, 13 Jun 2024 16:30:24 -0400 Subject: [PATCH 07/17] update constructor to take owner --- contracts/hooks/examples/FeeTaking.sol | 2 +- test/FeeTaking.t.sol | 10 +++++++--- test/shared/implementation/FeeTakingImplementation.sol | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/contracts/hooks/examples/FeeTaking.sol b/contracts/hooks/examples/FeeTaking.sol index 07b04777..183325dd 100644 --- a/contracts/hooks/examples/FeeTaking.sol +++ b/contracts/hooks/examples/FeeTaking.sol @@ -24,7 +24,7 @@ contract FeeTaking is BaseHook, IUnlockCallback, Owned { Currency[] currencies; } - constructor(IPoolManager _poolManager, uint128 _swapFeeBips) BaseHook(_poolManager) Owned(msg.sender) { + constructor(IPoolManager _poolManager, uint128 _swapFeeBips, address _owner) BaseHook(_poolManager) Owned(_owner) { swapFeeBips = _swapFeeBips; } diff --git a/test/FeeTaking.t.sol b/test/FeeTaking.t.sol index 028c7ebe..b301cfc9 100644 --- a/test/FeeTaking.t.sol +++ b/test/FeeTaking.t.sol @@ -43,7 +43,7 @@ contract FeeTakingTest is Test, Deployers { token1 = TestERC20(Currency.unwrap(currency1)); vm.record(); - FeeTakingImplementation impl = new FeeTakingImplementation(manager, 25, feeTaking); + FeeTakingImplementation impl = new FeeTakingImplementation(manager, 25, address(this), feeTaking); (, bytes32[] memory writes) = vm.accesses(address(impl)); vm.etch(address(feeTaking), address(impl).code); // for each storage key that was written during the hook implementation, copy the value over @@ -100,12 +100,13 @@ contract FeeTakingTest is Test, Deployers { currencies[0] = key.currency0; currencies[1] = key.currency1; feeTaking.withdraw(TREASURY, currencies); - assertEq(manager.balanceOf(address(this), CurrencyLibrary.toId(key.currency0)), 0); - assertEq(manager.balanceOf(address(this), CurrencyLibrary.toId(key.currency1)), 0); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)), 0); assertEq(currency0.balanceOf(TREASURY) / R, expectedFee2 / R); assertEq(currency1.balanceOf(TREASURY) / R, expectedFee / R); } + // this would error had the hook not used ERC6909 function testEdgeCase() public { // first, deplete the pool of token1 // Swap exact token0 for token1 // @@ -113,6 +114,9 @@ contract FeeTakingTest is Test, Deployers { int256 amountSpecified = -1e18; BalanceDelta swapDelta = swap(key, zeroForOne, amountSpecified, ZERO_BYTES); // ---------------------------- // + // now, pool only has 1 wei of token1 + uint256 poolToken1 = currency1.balanceOf(address(manager)) - manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)); + assertEq(poolToken1, 1); uint128 output = uint128(swapDelta.amount1()); assertTrue(output > 0); diff --git a/test/shared/implementation/FeeTakingImplementation.sol b/test/shared/implementation/FeeTakingImplementation.sol index 1d3bc051..697dbf41 100644 --- a/test/shared/implementation/FeeTakingImplementation.sol +++ b/test/shared/implementation/FeeTakingImplementation.sol @@ -7,8 +7,8 @@ import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; contract FeeTakingImplementation is FeeTaking { - constructor(IPoolManager _poolManager, uint128 _swapFeeBips, FeeTaking addressToEtch) - FeeTaking(_poolManager, _swapFeeBips) + constructor(IPoolManager _poolManager, uint128 _swapFeeBips, address _owner, FeeTaking addressToEtch) + FeeTaking(_poolManager, _swapFeeBips, _owner) { Hooks.validateHookPermissions(addressToEtch, getHookPermissions()); } From a526b2d0cad05f4ae0ccbc2edc31e3c8d4ad7d1d Mon Sep 17 00:00:00 2001 From: Junion Date: Thu, 13 Jun 2024 17:13:21 -0400 Subject: [PATCH 08/17] add more tests --- contracts/hooks/examples/FeeTaking.sol | 3 +- test/FeeTaking.t.sol | 123 ++++++++++++++++++++++++- 2 files changed, 120 insertions(+), 6 deletions(-) diff --git a/contracts/hooks/examples/FeeTaking.sol b/contracts/hooks/examples/FeeTaking.sol index 183325dd..942f1f3e 100644 --- a/contracts/hooks/examples/FeeTaking.sol +++ b/contracts/hooks/examples/FeeTaking.sol @@ -15,6 +15,7 @@ import {IUnlockCallback} from "@uniswap/v4-core/src/interfaces/callback/IUnlockC contract FeeTaking is BaseHook, IUnlockCallback, Owned { using SafeCast for uint256; + bytes internal constant ZERO_BYTES = bytes(""); uint128 private constant TOTAL_BIPS = 10000; uint128 private constant MAX_BIPS = 100; uint128 public swapFeeBips; @@ -93,6 +94,6 @@ contract FeeTaking is BaseHook, IUnlockCallback, Owned { i++; } } - return ""; + return ZERO_BYTES; } } diff --git a/test/FeeTaking.t.sol b/test/FeeTaking.t.sol index b301cfc9..85de2605 100644 --- a/test/FeeTaking.t.sol +++ b/test/FeeTaking.t.sol @@ -76,7 +76,7 @@ contract FeeTakingTest is Test, Deployers { uint128 output = uint128(swapDelta.amount1()); assertTrue(output > 0); - uint256 expectedFee = output * TOTAL_BIPS / (TOTAL_BIPS - feeTaking.swapFeeBips()) - output; + uint256 expectedFee = calculateFeeForExactInput(output, feeTaking.swapFeeBips()); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); @@ -90,7 +90,7 @@ contract FeeTakingTest is Test, Deployers { uint128 input = uint128(-swapDelta2.amount0()); assertTrue(output > 0); - uint128 expectedFee2 = (input * feeTaking.swapFeeBips()) / (TOTAL_BIPS + feeTaking.swapFeeBips()); + uint256 expectedFee2 = calculateFeeForExactOutput(input, feeTaking.swapFeeBips()); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)) / R, expectedFee2 / R); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); @@ -115,13 +115,14 @@ contract FeeTakingTest is Test, Deployers { BalanceDelta swapDelta = swap(key, zeroForOne, amountSpecified, ZERO_BYTES); // ---------------------------- // // now, pool only has 1 wei of token1 - uint256 poolToken1 = currency1.balanceOf(address(manager)) - manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)); + uint256 poolToken1 = currency1.balanceOf(address(manager)) + - manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)); assertEq(poolToken1, 1); uint128 output = uint128(swapDelta.amount1()); assertTrue(output > 0); - uint256 expectedFee = output * TOTAL_BIPS / (TOTAL_BIPS - feeTaking.swapFeeBips()) - output; + uint256 expectedFee = calculateFeeForExactInput(output, feeTaking.swapFeeBips()); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); @@ -135,7 +136,7 @@ contract FeeTakingTest is Test, Deployers { uint128 input = uint128(-swapDelta2.amount1()); assertTrue(output > 0); - uint128 expectedFee2 = (input * feeTaking.swapFeeBips()) / (TOTAL_BIPS + feeTaking.swapFeeBips()); + uint256 expectedFee2 = calculateFeeForExactOutput(input, feeTaking.swapFeeBips()); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); assertEq( @@ -151,4 +152,116 @@ contract FeeTakingTest is Test, Deployers { assertEq(currency0.balanceOf(TREASURY) / R, 0); assertEq(currency1.balanceOf(TREASURY) / R, (expectedFee + expectedFee2) / R); } + + function testSwapWithDifferentFees() public { + testSwapHooks(); + feeTaking.setSwapFeeBips(50); // Set fee to 0.50% + + // Swap exact token0 for token1 // + bool zeroForOne = true; + int256 amountSpecified = -1e12; + BalanceDelta swapDelta = swap(key, zeroForOne, amountSpecified, ZERO_BYTES); + // ---------------------------- // + + uint128 output = uint128(swapDelta.amount1()); + assertTrue(output > 0); + + uint256 expectedFee = calculateFeeForExactInput(output, 50); + + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); + } + + function testZeroFeeSwap() public { + feeTaking.setSwapFeeBips(0); // Set fee to 0% + + // Swap exact token0 for token1 // + bool zeroForOne = true; + int256 amountSpecified = -1e12; + BalanceDelta swapDelta = swap(key, zeroForOne, amountSpecified, ZERO_BYTES); + // ---------------------------- // + + uint128 output = uint128(swapDelta.amount1()); + assertTrue(output > 0); + + // No fee should be taken + uint256 expectedFee = calculateFeeForExactInput(output, 0); + assertEq(expectedFee, 0); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)), 0); + } + + function testMaxFeeSwap() public { + feeTaking.setSwapFeeBips(100); // Set fee to 1% + + // Swap exact token0 for token1 // + bool zeroForOne = true; + int256 amountSpecified = -1e12; + BalanceDelta swapDelta = swap(key, zeroForOne, amountSpecified, ZERO_BYTES); + // ---------------------------- // + + uint128 output = uint128(swapDelta.amount1()); + assertTrue(output > 0); + + uint256 expectedFee = calculateFeeForExactInput(output, 100); + + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); + } + + function testMultiTokenPoolSwap() public { + testSwapHooks(); + // Deploy additional tokens + (Currency currency2, Currency currency3) = deployMintAndApprove2Currencies(); + TestERC20 token2 = TestERC20(Currency.unwrap(currency2)); + TestERC20 token3 = TestERC20(Currency.unwrap(currency3)); + + // Create new pool with different tokens + (PoolKey memory key2, PoolId id2) = + initPoolAndAddLiquidity(currency2, currency3, feeTaking, 3000, SQRT_RATIO_10_1, ZERO_BYTES); + + // Approve tokens for the router + token2.approve(address(router), type(uint256).max); + token3.approve(address(router), type(uint256).max); + + // Swap exact token2 for token3 // + bool zeroForOne = true; + int256 amountSpecified = -1e12; + BalanceDelta swapDelta = swap(key2, zeroForOne, amountSpecified, ZERO_BYTES); + // ---------------------------- // + + uint128 output = uint128(swapDelta.amount1()); + assertTrue(output > 0); + + uint256 expectedFee = calculateFeeForExactInput(output, feeTaking.swapFeeBips()); + + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key2.currency0)), 0); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key2.currency1)) / R, expectedFee / R); + + // Withdraw accumulated fees + Currency[] memory currencies = new Currency[](3); + currencies[0] = key.currency0; + currencies[1] = key.currency1; + currencies[2] = key2.currency1; + feeTaking.withdraw(TREASURY, currencies); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)), 0); + assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key2.currency1)), 0); + assertEq(currency0.balanceOf(TREASURY) / R, 0); + assertEq(currency1.balanceOf(TREASURY) / R, expectedFee / R); + assertEq(currency3.balanceOf(TREASURY) / R, expectedFee / R); + } + + function testTooHighFee() public { + vm.expectRevert(); + feeTaking.setSwapFeeBips(101); + } + + function calculateFeeForExactInput(uint256 outputAmount, uint128 feeBips) internal pure returns (uint256) { + return outputAmount * TOTAL_BIPS / (TOTAL_BIPS - feeBips) - outputAmount; + } + + function calculateFeeForExactOutput(uint256 inputAmount, uint128 feeBips) internal pure returns (uint256) { + return (inputAmount * feeBips) / (TOTAL_BIPS + feeBips); + } } From 852defc8b3fa26dc74ccdc5fa9b2d4895282cd14 Mon Sep 17 00:00:00 2001 From: Sara Reynolds <30504811+snreynolds@users.noreply.github.com> Date: Tue, 18 Jun 2024 11:53:46 -0400 Subject: [PATCH 09/17] add safecallback (#123) * add safecallback * use manager --- .../FullRangeAddInitialLiquidity.snap | 2 +- .forge-snapshots/FullRangeAddLiquidity.snap | 2 +- .forge-snapshots/FullRangeFirstSwap.snap | 2 +- .forge-snapshots/FullRangeInitialize.snap | 2 +- .../FullRangeRemoveLiquidity.snap | 2 +- .../FullRangeRemoveLiquidityAndRebalance.snap | 2 +- .forge-snapshots/FullRangeSecondSwap.snap | 2 +- .forge-snapshots/FullRangeSwap.snap | 2 +- .forge-snapshots/TWAMMSubmitOrder.snap | 2 +- contracts/BaseHook.sol | 19 ++---- contracts/base/ImmutableState.sol | 12 ++++ contracts/base/SafeCallback.sol | 22 +++++++ contracts/hooks/examples/FullRange.sol | 45 ++++++-------- contracts/hooks/examples/GeomeanOracle.sol | 24 ++++---- contracts/hooks/examples/LimitOrder.sol | 50 +++++++-------- contracts/hooks/examples/TWAMM.sol | 61 +++++++++---------- contracts/hooks/examples/VolatilityOracle.sol | 4 +- 17 files changed, 136 insertions(+), 119 deletions(-) create mode 100644 contracts/base/ImmutableState.sol create mode 100644 contracts/base/SafeCallback.sol diff --git a/.forge-snapshots/FullRangeAddInitialLiquidity.snap b/.forge-snapshots/FullRangeAddInitialLiquidity.snap index cd1e3c37..404cf12a 100644 --- a/.forge-snapshots/FullRangeAddInitialLiquidity.snap +++ b/.forge-snapshots/FullRangeAddInitialLiquidity.snap @@ -1 +1 @@ -311073 \ No newline at end of file +311181 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeAddLiquidity.snap b/.forge-snapshots/FullRangeAddLiquidity.snap index b3de2b4e..a4a14676 100644 --- a/.forge-snapshots/FullRangeAddLiquidity.snap +++ b/.forge-snapshots/FullRangeAddLiquidity.snap @@ -1 +1 @@ -122882 \ No newline at end of file +122990 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeFirstSwap.snap b/.forge-snapshots/FullRangeFirstSwap.snap index 54d0d097..da120795 100644 --- a/.forge-snapshots/FullRangeFirstSwap.snap +++ b/.forge-snapshots/FullRangeFirstSwap.snap @@ -1 +1 @@ -80283 \ No newline at end of file +80220 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeInitialize.snap b/.forge-snapshots/FullRangeInitialize.snap index f81651f8..7a0170eb 100644 --- a/.forge-snapshots/FullRangeInitialize.snap +++ b/.forge-snapshots/FullRangeInitialize.snap @@ -1 +1 @@ -1015169 \ No newline at end of file +1015181 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeRemoveLiquidity.snap b/.forge-snapshots/FullRangeRemoveLiquidity.snap index 265c1dec..feea4936 100644 --- a/.forge-snapshots/FullRangeRemoveLiquidity.snap +++ b/.forge-snapshots/FullRangeRemoveLiquidity.snap @@ -1 +1 @@ -110476 \ No newline at end of file +110566 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeRemoveLiquidityAndRebalance.snap b/.forge-snapshots/FullRangeRemoveLiquidityAndRebalance.snap index dcb62527..e0df7eb7 100644 --- a/.forge-snapshots/FullRangeRemoveLiquidityAndRebalance.snap +++ b/.forge-snapshots/FullRangeRemoveLiquidityAndRebalance.snap @@ -1 +1 @@ -239954 \ No newline at end of file +240044 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeSecondSwap.snap b/.forge-snapshots/FullRangeSecondSwap.snap index 1bf183ef..e68df8d3 100644 --- a/.forge-snapshots/FullRangeSecondSwap.snap +++ b/.forge-snapshots/FullRangeSecondSwap.snap @@ -1 +1 @@ -45993 \ No newline at end of file +45930 \ No newline at end of file diff --git a/.forge-snapshots/FullRangeSwap.snap b/.forge-snapshots/FullRangeSwap.snap index 5630ac05..b50d0ea2 100644 --- a/.forge-snapshots/FullRangeSwap.snap +++ b/.forge-snapshots/FullRangeSwap.snap @@ -1 +1 @@ -79414 \ No newline at end of file +79351 \ No newline at end of file diff --git a/.forge-snapshots/TWAMMSubmitOrder.snap b/.forge-snapshots/TWAMMSubmitOrder.snap index b2759d7f..eb3b0f6b 100644 --- a/.forge-snapshots/TWAMMSubmitOrder.snap +++ b/.forge-snapshots/TWAMMSubmitOrder.snap @@ -1 +1 @@ -122355 \ No newline at end of file +122336 \ No newline at end of file diff --git a/contracts/BaseHook.sol b/contracts/BaseHook.sol index eb75502c..01fc4954 100644 --- a/contracts/BaseHook.sol +++ b/contracts/BaseHook.sol @@ -7,28 +7,19 @@ import {IHooks} from "@uniswap/v4-core/src/interfaces/IHooks.sol"; import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; import {BeforeSwapDelta} from "@uniswap/v4-core/src/types/BeforeSwapDelta.sol"; +import {SafeCallback} from "./base/SafeCallback.sol"; +import {ImmutableState} from "./base/ImmutableState.sol"; -abstract contract BaseHook is IHooks { - error NotPoolManager(); +abstract contract BaseHook is IHooks, SafeCallback { error NotSelf(); error InvalidPool(); error LockFailure(); error HookNotImplemented(); - /// @notice The address of the pool manager - IPoolManager public immutable poolManager; - - constructor(IPoolManager _poolManager) { - poolManager = _poolManager; + constructor(IPoolManager _manager) ImmutableState(_manager) { validateHookAddress(this); } - /// @dev Only the pool manager may call this function - modifier poolManagerOnly() { - if (msg.sender != address(poolManager)) revert NotPoolManager(); - _; - } - /// @dev Only this address may call this function modifier selfOnly() { if (msg.sender != address(this)) revert NotSelf(); @@ -50,7 +41,7 @@ abstract contract BaseHook is IHooks { Hooks.validateHookPermissions(_this, getHookPermissions()); } - function unlockCallback(bytes calldata data) external virtual poolManagerOnly returns (bytes memory) { + function _unlockCallback(bytes calldata data) internal virtual override returns (bytes memory) { (bool success, bytes memory returnData) = address(this).call(data); if (success) return returnData; if (returnData.length == 0) revert LockFailure(); diff --git a/contracts/base/ImmutableState.sol b/contracts/base/ImmutableState.sol new file mode 100644 index 00000000..cce37514 --- /dev/null +++ b/contracts/base/ImmutableState.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +pragma solidity ^0.8.19; + +import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; + +contract ImmutableState { + IPoolManager public immutable manager; + + constructor(IPoolManager _manager) { + manager = _manager; + } +} diff --git a/contracts/base/SafeCallback.sol b/contracts/base/SafeCallback.sol new file mode 100644 index 00000000..f985e67c --- /dev/null +++ b/contracts/base/SafeCallback.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.19; + +import {IUnlockCallback} from "@uniswap/v4-core/src/interfaces/callback/IUnlockCallback.sol"; +import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; +import {ImmutableState} from "./ImmutableState.sol"; + +abstract contract SafeCallback is ImmutableState, IUnlockCallback { + error NotManager(); + + modifier onlyByManager() { + if (msg.sender != address(manager)) revert NotManager(); + _; + } + + /// @dev We force the onlyByManager modifier by exposing a virtual function after the onlyByManager check. + function unlockCallback(bytes calldata data) external onlyByManager returns (bytes memory) { + return _unlockCallback(data); + } + + function _unlockCallback(bytes calldata data) internal virtual returns (bytes memory); +} diff --git a/contracts/hooks/examples/FullRange.sol b/contracts/hooks/examples/FullRange.sol index 194be803..191593b8 100644 --- a/contracts/hooks/examples/FullRange.sol +++ b/contracts/hooks/examples/FullRange.sol @@ -26,7 +26,7 @@ import {BeforeSwapDelta, BeforeSwapDeltaLibrary} from "@uniswap/v4-core/src/type import "../../libraries/LiquidityAmounts.sol"; -contract FullRange is BaseHook, IUnlockCallback { +contract FullRange is BaseHook { using CurrencyLibrary for Currency; using CurrencySettler for Currency; using PoolIdLibrary for PoolKey; @@ -85,7 +85,7 @@ contract FullRange is BaseHook, IUnlockCallback { mapping(PoolId => PoolInfo) public poolInfo; - constructor(IPoolManager _poolManager) BaseHook(_poolManager) {} + constructor(IPoolManager _manager) BaseHook(_manager) {} modifier ensure(uint256 deadline) { if (deadline < block.timestamp) revert ExpiredPastDeadline(); @@ -126,13 +126,13 @@ contract FullRange is BaseHook, IUnlockCallback { PoolId poolId = key.toId(); - (uint160 sqrtPriceX96,,,) = poolManager.getSlot0(poolId); + (uint160 sqrtPriceX96,,,) = manager.getSlot0(poolId); if (sqrtPriceX96 == 0) revert PoolNotInitialized(); PoolInfo storage pool = poolInfo[poolId]; - uint128 poolLiquidity = poolManager.getLiquidity(poolId); + uint128 poolLiquidity = manager.getLiquidity(poolId); liquidity = LiquidityAmounts.getLiquidityForAmounts( sqrtPriceX96, @@ -184,7 +184,7 @@ contract FullRange is BaseHook, IUnlockCallback { PoolId poolId = key.toId(); - (uint160 sqrtPriceX96,,,) = poolManager.getSlot0(poolId); + (uint160 sqrtPriceX96,,,) = manager.getSlot0(poolId); if (sqrtPriceX96 == 0) revert PoolNotInitialized(); @@ -260,17 +260,17 @@ contract FullRange is BaseHook, IUnlockCallback { internal returns (BalanceDelta delta) { - delta = abi.decode(poolManager.unlock(abi.encode(CallbackData(msg.sender, key, params))), (BalanceDelta)); + delta = abi.decode(manager.unlock(abi.encode(CallbackData(msg.sender, key, params))), (BalanceDelta)); } function _settleDeltas(address sender, PoolKey memory key, BalanceDelta delta) internal { - key.currency0.settle(poolManager, sender, uint256(int256(-delta.amount0())), false); - key.currency1.settle(poolManager, sender, uint256(int256(-delta.amount1())), false); + key.currency0.settle(manager, sender, uint256(int256(-delta.amount0())), false); + key.currency1.settle(manager, sender, uint256(int256(-delta.amount1())), false); } function _takeDeltas(address sender, PoolKey memory key, BalanceDelta delta) internal { - poolManager.take(key.currency0, sender, uint256(uint128(delta.amount0()))); - poolManager.take(key.currency1, sender, uint256(uint128(delta.amount1()))); + manager.take(key.currency0, sender, uint256(uint128(delta.amount0()))); + manager.take(key.currency1, sender, uint256(uint128(delta.amount1()))); } function _removeLiquidity(PoolKey memory key, IPoolManager.ModifyLiquidityParams memory params) @@ -286,21 +286,16 @@ contract FullRange is BaseHook, IUnlockCallback { uint256 liquidityToRemove = FullMath.mulDiv( uint256(-params.liquidityDelta), - poolManager.getLiquidity(poolId), + manager.getLiquidity(poolId), UniswapV4ERC20(pool.liquidityToken).totalSupply() ); params.liquidityDelta = -(liquidityToRemove.toInt256()); - (delta,) = poolManager.modifyLiquidity(key, params, ZERO_BYTES); + (delta,) = manager.modifyLiquidity(key, params, ZERO_BYTES); pool.hasAccruedFees = false; } - function unlockCallback(bytes calldata rawData) - external - override(IUnlockCallback, BaseHook) - poolManagerOnly - returns (bytes memory) - { + function _unlockCallback(bytes calldata rawData) internal override returns (bytes memory) { CallbackData memory data = abi.decode(rawData, (CallbackData)); BalanceDelta delta; @@ -308,7 +303,7 @@ contract FullRange is BaseHook, IUnlockCallback { delta = _removeLiquidity(data.key, data.params); _takeDeltas(data.sender, data.key, delta); } else { - (delta,) = poolManager.modifyLiquidity(data.key, data.params, ZERO_BYTES); + (delta,) = manager.modifyLiquidity(data.key, data.params, ZERO_BYTES); _settleDeltas(data.sender, data.key, delta); } return abi.encode(delta); @@ -316,12 +311,12 @@ contract FullRange is BaseHook, IUnlockCallback { function _rebalance(PoolKey memory key) public { PoolId poolId = key.toId(); - (BalanceDelta balanceDelta,) = poolManager.modifyLiquidity( + (BalanceDelta balanceDelta,) = manager.modifyLiquidity( key, IPoolManager.ModifyLiquidityParams({ tickLower: MIN_TICK, tickUpper: MAX_TICK, - liquidityDelta: -(poolManager.getLiquidity(poolId).toInt256()), + liquidityDelta: -(manager.getLiquidity(poolId).toInt256()), salt: 0 }), ZERO_BYTES @@ -333,9 +328,9 @@ contract FullRange is BaseHook, IUnlockCallback { ) * FixedPointMathLib.sqrt(FixedPoint96.Q96) ).toUint160(); - (uint160 sqrtPriceX96,,,) = poolManager.getSlot0(poolId); + (uint160 sqrtPriceX96,,,) = manager.getSlot0(poolId); - poolManager.swap( + manager.swap( key, IPoolManager.SwapParams({ zeroForOne: newSqrtPriceX96 < sqrtPriceX96, @@ -353,7 +348,7 @@ contract FullRange is BaseHook, IUnlockCallback { uint256(uint128(balanceDelta.amount1())) ); - (BalanceDelta balanceDeltaAfter,) = poolManager.modifyLiquidity( + (BalanceDelta balanceDeltaAfter,) = manager.modifyLiquidity( key, IPoolManager.ModifyLiquidityParams({ tickLower: MIN_TICK, @@ -368,6 +363,6 @@ contract FullRange is BaseHook, IUnlockCallback { uint128 donateAmount0 = uint128(balanceDelta.amount0() + balanceDeltaAfter.amount0()); uint128 donateAmount1 = uint128(balanceDelta.amount1() + balanceDeltaAfter.amount1()); - poolManager.donate(key, donateAmount0, donateAmount1, ZERO_BYTES); + manager.donate(key, donateAmount0, donateAmount1, ZERO_BYTES); } } diff --git a/contracts/hooks/examples/GeomeanOracle.sol b/contracts/hooks/examples/GeomeanOracle.sol index ec8301a5..df5a9ad1 100644 --- a/contracts/hooks/examples/GeomeanOracle.sol +++ b/contracts/hooks/examples/GeomeanOracle.sol @@ -61,7 +61,7 @@ contract GeomeanOracle is BaseHook { return uint32(block.timestamp); } - constructor(IPoolManager _poolManager) BaseHook(_poolManager) {} + constructor(IPoolManager _manager) BaseHook(_manager) {} function getHookPermissions() public pure override returns (Hooks.Permissions memory) { return Hooks.Permissions({ @@ -86,20 +86,20 @@ contract GeomeanOracle is BaseHook { external view override - poolManagerOnly + onlyByManager returns (bytes4) { // This is to limit the fragmentation of pools using this oracle hook. In other words, // there may only be one pool per pair of tokens that use this hook. The tick spacing is set to the maximum // because we only allow max range liquidity in this pool. - if (key.fee != 0 || key.tickSpacing != poolManager.MAX_TICK_SPACING()) revert OnlyOneOraclePoolAllowed(); + if (key.fee != 0 || key.tickSpacing != manager.MAX_TICK_SPACING()) revert OnlyOneOraclePoolAllowed(); return GeomeanOracle.beforeInitialize.selector; } function afterInitialize(address, PoolKey calldata key, uint160, int24, bytes calldata) external override - poolManagerOnly + onlyByManager returns (bytes4) { PoolId id = key.toId(); @@ -110,9 +110,9 @@ contract GeomeanOracle is BaseHook { /// @dev Called before any action that potentially modifies pool price or liquidity, such as swap or modify position function _updatePool(PoolKey calldata key) private { PoolId id = key.toId(); - (, int24 tick,,) = poolManager.getSlot0(id); + (, int24 tick,,) = manager.getSlot0(id); - uint128 liquidity = poolManager.getLiquidity(id); + uint128 liquidity = manager.getLiquidity(id); (states[id].index, states[id].cardinality) = observations[id].write( states[id].index, _blockTimestamp(), tick, liquidity, states[id].cardinality, states[id].cardinalityNext @@ -124,8 +124,8 @@ contract GeomeanOracle is BaseHook { PoolKey calldata key, IPoolManager.ModifyLiquidityParams calldata params, bytes calldata - ) external override poolManagerOnly returns (bytes4) { - int24 maxTickSpacing = poolManager.MAX_TICK_SPACING(); + ) external override onlyByManager returns (bytes4) { + int24 maxTickSpacing = manager.MAX_TICK_SPACING(); if ( params.tickLower != TickMath.minUsableTick(maxTickSpacing) || params.tickUpper != TickMath.maxUsableTick(maxTickSpacing) @@ -139,14 +139,14 @@ contract GeomeanOracle is BaseHook { PoolKey calldata, IPoolManager.ModifyLiquidityParams calldata, bytes calldata - ) external view override poolManagerOnly returns (bytes4) { + ) external view override onlyByManager returns (bytes4) { revert OraclePoolMustLockLiquidity(); } function beforeSwap(address, PoolKey calldata key, IPoolManager.SwapParams calldata, bytes calldata) external override - poolManagerOnly + onlyByManager returns (bytes4, BeforeSwapDelta, uint24) { _updatePool(key); @@ -163,9 +163,9 @@ contract GeomeanOracle is BaseHook { ObservationState memory state = states[id]; - (, int24 tick,,) = poolManager.getSlot0(id); + (, int24 tick,,) = manager.getSlot0(id); - uint128 liquidity = poolManager.getLiquidity(id); + uint128 liquidity = manager.getLiquidity(id); return observations[id].observe(_blockTimestamp(), secondsAgos, tick, state.index, liquidity, state.cardinality); } diff --git a/contracts/hooks/examples/LimitOrder.sol b/contracts/hooks/examples/LimitOrder.sol index 9ee7a33c..2a8ca909 100644 --- a/contracts/hooks/examples/LimitOrder.sol +++ b/contracts/hooks/examples/LimitOrder.sol @@ -75,7 +75,7 @@ contract LimitOrder is BaseHook { mapping(bytes32 => Epoch) public epochs; mapping(Epoch => EpochInfo) public epochInfos; - constructor(IPoolManager _poolManager) BaseHook(_poolManager) {} + constructor(IPoolManager _manager) BaseHook(_manager) {} function getHookPermissions() public pure override returns (Hooks.Permissions memory) { return Hooks.Permissions({ @@ -117,7 +117,7 @@ contract LimitOrder is BaseHook { } function getTick(PoolId poolId) private view returns (int24 tick) { - (, tick,,) = poolManager.getSlot0(poolId); + (, tick,,) = manager.getSlot0(poolId); } function getTickLower(int24 tick, int24 tickSpacing) private pure returns (int24) { @@ -129,7 +129,7 @@ contract LimitOrder is BaseHook { function afterInitialize(address, PoolKey calldata key, uint160, int24 tick, bytes calldata) external override - poolManagerOnly + onlyByManager returns (bytes4) { setTickLowerLast(key.toId(), getTickLower(tick, key.tickSpacing)); @@ -142,7 +142,7 @@ contract LimitOrder is BaseHook { IPoolManager.SwapParams calldata params, BalanceDelta, bytes calldata - ) external override poolManagerOnly returns (bytes4, int128) { + ) external override onlyByManager returns (bytes4, int128) { (int24 tickLower, int24 lower, int24 upper) = _getCrossedTicks(key.toId(), key.tickSpacing); if (lower > upper) return (LimitOrder.afterSwap.selector, 0); @@ -197,10 +197,10 @@ contract LimitOrder is BaseHook { function _unlockCallbackFill(PoolKey calldata key, int24 tickLower, int256 liquidityDelta) private - poolManagerOnly + onlyByManager returns (uint128 amount0, uint128 amount1) { - (BalanceDelta delta,) = poolManager.modifyLiquidity( + (BalanceDelta delta,) = manager.modifyLiquidity( key, IPoolManager.ModifyLiquidityParams({ tickLower: tickLower, @@ -212,10 +212,10 @@ contract LimitOrder is BaseHook { ); if (delta.amount0() > 0) { - poolManager.mint(address(this), key.currency0.toId(), amount0 = uint128(delta.amount0())); + manager.mint(address(this), key.currency0.toId(), amount0 = uint128(delta.amount0())); } if (delta.amount1() > 0) { - poolManager.mint(address(this), key.currency1.toId(), amount1 = uint128(delta.amount1())); + manager.mint(address(this), key.currency1.toId(), amount1 = uint128(delta.amount1())); } } @@ -225,7 +225,7 @@ contract LimitOrder is BaseHook { { if (liquidity == 0) revert ZeroLiquidity(); - poolManager.unlock( + manager.unlock( abi.encodeCall( this.unlockCallbackPlace, (key, tickLower, zeroForOne, int256(uint256(liquidity)), msg.sender) ) @@ -263,7 +263,7 @@ contract LimitOrder is BaseHook { int256 liquidityDelta, address owner ) external selfOnly { - (BalanceDelta delta,) = poolManager.modifyLiquidity( + (BalanceDelta delta,) = manager.modifyLiquidity( key, IPoolManager.ModifyLiquidityParams({ tickLower: tickLower, @@ -277,11 +277,11 @@ contract LimitOrder is BaseHook { if (delta.amount0() < 0) { if (delta.amount1() != 0) revert InRange(); if (!zeroForOne) revert CrossedRange(); - key.currency0.settle(poolManager, owner, uint256(uint128(-delta.amount0())), false); + key.currency0.settle(manager, owner, uint256(uint128(-delta.amount0())), false); } else { if (delta.amount0() != 0) revert InRange(); if (zeroForOne) revert CrossedRange(); - key.currency1.settle(poolManager, owner, uint256(uint128(-delta.amount1())), false); + key.currency1.settle(manager, owner, uint256(uint128(-delta.amount1())), false); } } @@ -298,7 +298,7 @@ contract LimitOrder is BaseHook { uint256 amount0Fee; uint256 amount1Fee; (amount0Fee, amount1Fee) = abi.decode( - poolManager.unlock( + manager.unlock( abi.encodeCall( this.unlockCallbackKill, (key, tickLower, -int256(uint256(liquidity)), to, liquidity == epochInfo.liquidityTotal) @@ -329,7 +329,7 @@ contract LimitOrder is BaseHook { // could be unfairly diluted by a user sychronously placing then killing a limit order to skim off fees. // to prevent this, we allocate all fee revenue to remaining limit order placers, unless this is the last order. if (!removingAllLiquidity) { - (, BalanceDelta deltaFee) = poolManager.modifyLiquidity( + (, BalanceDelta deltaFee) = manager.modifyLiquidity( key, IPoolManager.ModifyLiquidityParams({ tickLower: tickLower, @@ -341,14 +341,14 @@ contract LimitOrder is BaseHook { ); if (deltaFee.amount0() > 0) { - poolManager.mint(address(this), key.currency0.toId(), amount0Fee = uint128(deltaFee.amount0())); + manager.mint(address(this), key.currency0.toId(), amount0Fee = uint128(deltaFee.amount0())); } if (deltaFee.amount1() > 0) { - poolManager.mint(address(this), key.currency1.toId(), amount1Fee = uint128(deltaFee.amount1())); + manager.mint(address(this), key.currency1.toId(), amount1Fee = uint128(deltaFee.amount1())); } } - (BalanceDelta delta,) = poolManager.modifyLiquidity( + (BalanceDelta delta,) = manager.modifyLiquidity( key, IPoolManager.ModifyLiquidityParams({ tickLower: tickLower, @@ -360,10 +360,10 @@ contract LimitOrder is BaseHook { ); if (delta.amount0() > 0) { - key.currency0.take(poolManager, to, uint256(uint128(delta.amount0())), false); + key.currency0.take(manager, to, uint256(uint128(delta.amount0())), false); } if (delta.amount1() > 0) { - key.currency1.take(poolManager, to, uint256(uint128(delta.amount1())), false); + key.currency1.take(manager, to, uint256(uint128(delta.amount1())), false); } } @@ -385,7 +385,7 @@ contract LimitOrder is BaseHook { epochInfo.token1Total -= amount1; epochInfo.liquidityTotal = liquidityTotal - liquidity; - poolManager.unlock( + manager.unlock( abi.encodeCall( this.unlockCallbackWithdraw, (epochInfo.currency0, epochInfo.currency1, amount0, amount1, to) ) @@ -402,17 +402,17 @@ contract LimitOrder is BaseHook { address to ) external selfOnly { if (token0Amount > 0) { - poolManager.burn(address(this), currency0.toId(), token0Amount); - poolManager.take(currency0, to, token0Amount); + manager.burn(address(this), currency0.toId(), token0Amount); + manager.take(currency0, to, token0Amount); } if (token1Amount > 0) { - poolManager.burn(address(this), currency1.toId(), token1Amount); - poolManager.take(currency1, to, token1Amount); + manager.burn(address(this), currency1.toId(), token1Amount); + manager.take(currency1, to, token1Amount); } } function onERC1155Received(address, address, uint256, uint256, bytes calldata) external view returns (bytes4) { - if (msg.sender != address(poolManager)) revert NotPoolManagerToken(); + if (msg.sender != address(manager)) revert NotPoolManagerToken(); return IERC1155Receiver.onERC1155Received.selector; } } diff --git a/contracts/hooks/examples/TWAMM.sol b/contracts/hooks/examples/TWAMM.sol index 5704d765..dc1f3b00 100644 --- a/contracts/hooks/examples/TWAMM.sol +++ b/contracts/hooks/examples/TWAMM.sol @@ -61,7 +61,7 @@ contract TWAMM is BaseHook, ITWAMM { // tokensOwed[token][owner] => amountOwed mapping(Currency => mapping(address => uint256)) public tokensOwed; - constructor(IPoolManager _poolManager, uint256 _expirationInterval) BaseHook(_poolManager) { + constructor(IPoolManager _manager, uint256 _expirationInterval) BaseHook(_manager) { expirationInterval = _expirationInterval; } @@ -88,7 +88,7 @@ contract TWAMM is BaseHook, ITWAMM { external virtual override - poolManagerOnly + onlyByManager returns (bytes4) { // one-time initialization enforced in PoolManager @@ -101,7 +101,7 @@ contract TWAMM is BaseHook, ITWAMM { PoolKey calldata key, IPoolManager.ModifyLiquidityParams calldata, bytes calldata - ) external override poolManagerOnly returns (bytes4) { + ) external override onlyByManager returns (bytes4) { executeTWAMMOrders(key); return BaseHook.beforeAddLiquidity.selector; } @@ -109,7 +109,7 @@ contract TWAMM is BaseHook, ITWAMM { function beforeSwap(address, PoolKey calldata key, IPoolManager.SwapParams calldata, bytes calldata) external override - poolManagerOnly + onlyByManager returns (bytes4, BeforeSwapDelta, uint24) { executeTWAMMOrders(key); @@ -143,17 +143,14 @@ contract TWAMM is BaseHook, ITWAMM { /// @inheritdoc ITWAMM function executeTWAMMOrders(PoolKey memory key) public { PoolId poolId = key.toId(); - (uint160 sqrtPriceX96,,,) = poolManager.getSlot0(poolId); + (uint160 sqrtPriceX96,,,) = manager.getSlot0(poolId); State storage twamm = twammStates[poolId]; - (bool zeroForOne, uint160 sqrtPriceLimitX96) = _executeTWAMMOrders( - twamm, poolManager, key, PoolParamsOnExecute(sqrtPriceX96, poolManager.getLiquidity(poolId)) - ); + (bool zeroForOne, uint160 sqrtPriceLimitX96) = + _executeTWAMMOrders(twamm, manager, key, PoolParamsOnExecute(sqrtPriceX96, manager.getLiquidity(poolId))); if (sqrtPriceLimitX96 != 0 && sqrtPriceLimitX96 != sqrtPriceX96) { - poolManager.unlock( - abi.encode(key, IPoolManager.SwapParams(zeroForOne, type(int256).max, sqrtPriceLimitX96)) - ); + manager.unlock(abi.encode(key, IPoolManager.SwapParams(zeroForOne, type(int256).max, sqrtPriceLimitX96))); } } @@ -309,25 +306,25 @@ contract TWAMM is BaseHook, ITWAMM { IERC20Minimal(Currency.unwrap(token)).safeTransfer(to, amountTransferred); } - function unlockCallback(bytes calldata rawData) external override poolManagerOnly returns (bytes memory) { + function _unlockCallback(bytes calldata rawData) internal override returns (bytes memory) { (PoolKey memory key, IPoolManager.SwapParams memory swapParams) = abi.decode(rawData, (PoolKey, IPoolManager.SwapParams)); - BalanceDelta delta = poolManager.swap(key, swapParams, ZERO_BYTES); + BalanceDelta delta = manager.swap(key, swapParams, ZERO_BYTES); if (swapParams.zeroForOne) { if (delta.amount0() < 0) { - key.currency0.settle(poolManager, address(this), uint256(uint128(-delta.amount0())), false); + key.currency0.settle(manager, address(this), uint256(uint128(-delta.amount0())), false); } if (delta.amount1() > 0) { - key.currency1.take(poolManager, address(this), uint256(uint128(delta.amount1())), false); + key.currency1.take(manager, address(this), uint256(uint128(delta.amount1())), false); } } else { if (delta.amount1() < 0) { - key.currency1.settle(poolManager, address(this), uint256(uint128(-delta.amount1())), false); + key.currency1.settle(manager, address(this), uint256(uint128(-delta.amount1())), false); } if (delta.amount0() > 0) { - key.currency0.take(poolManager, address(this), uint256(uint128(delta.amount0())), false); + key.currency0.take(manager, address(this), uint256(uint128(delta.amount0())), false); } } return bytes(""); @@ -346,7 +343,7 @@ contract TWAMM is BaseHook, ITWAMM { /// @param pool The relevant state of the pool function _executeTWAMMOrders( State storage self, - IPoolManager poolManager, + IPoolManager manager, PoolKey memory key, PoolParamsOnExecute memory pool ) internal returns (bool zeroForOne, uint160 newSqrtPriceX96) { @@ -371,7 +368,7 @@ contract TWAMM is BaseHook, ITWAMM { if (orderPool0For1.sellRateCurrent != 0 && orderPool1For0.sellRateCurrent != 0) { pool = _advanceToNewTimestamp( self, - poolManager, + manager, key, AdvanceParams( expirationInterval, @@ -383,7 +380,7 @@ contract TWAMM is BaseHook, ITWAMM { } else { pool = _advanceTimestampForSinglePoolSell( self, - poolManager, + manager, key, AdvanceSingleParams( expirationInterval, @@ -405,14 +402,14 @@ contract TWAMM is BaseHook, ITWAMM { if (orderPool0For1.sellRateCurrent != 0 && orderPool1For0.sellRateCurrent != 0) { pool = _advanceToNewTimestamp( self, - poolManager, + manager, key, AdvanceParams(expirationInterval, block.timestamp, block.timestamp - prevTimestamp, pool) ); } else { pool = _advanceTimestampForSinglePoolSell( self, - poolManager, + manager, key, AdvanceSingleParams( expirationInterval, @@ -440,7 +437,7 @@ contract TWAMM is BaseHook, ITWAMM { function _advanceToNewTimestamp( State storage self, - IPoolManager poolManager, + IPoolManager manager, PoolKey memory poolKey, AdvanceParams memory params ) private returns (PoolParamsOnExecute memory) { @@ -462,13 +459,13 @@ contract TWAMM is BaseHook, ITWAMM { finalSqrtPriceX96 = TwammMath.getNewSqrtPriceX96(executionParams); (bool crossingInitializedTick, int24 tick) = - _isCrossingInitializedTick(params.pool, poolManager, poolKey, finalSqrtPriceX96); + _isCrossingInitializedTick(params.pool, manager, poolKey, finalSqrtPriceX96); unchecked { if (crossingInitializedTick) { uint256 secondsUntilCrossingX96; (params.pool, secondsUntilCrossingX96) = _advanceTimeThroughTickCrossing( self, - poolManager, + manager, poolKey, TickCrossingParams(tick, params.nextTimestamp, secondsElapsedX96, params.pool) ); @@ -503,7 +500,7 @@ contract TWAMM is BaseHook, ITWAMM { function _advanceTimestampForSinglePoolSell( State storage self, - IPoolManager poolManager, + IPoolManager manager, PoolKey memory poolKey, AdvanceSingleParams memory params ) private returns (PoolParamsOnExecute memory) { @@ -518,10 +515,10 @@ contract TWAMM is BaseHook, ITWAMM { ); (bool crossingInitializedTick, int24 tick) = - _isCrossingInitializedTick(params.pool, poolManager, poolKey, finalSqrtPriceX96); + _isCrossingInitializedTick(params.pool, manager, poolKey, finalSqrtPriceX96); if (crossingInitializedTick) { - (, int128 liquidityNetAtTick) = poolManager.getTickLiquidity(poolKey.toId(), tick); + (, int128 liquidityNetAtTick) = manager.getTickLiquidity(poolKey.toId(), tick); uint160 initializedSqrtPrice = TickMath.getSqrtPriceAtTick(tick); uint256 swapDelta0 = SqrtPriceMath.getAmount0Delta( @@ -575,7 +572,7 @@ contract TWAMM is BaseHook, ITWAMM { function _advanceTimeThroughTickCrossing( State storage self, - IPoolManager poolManager, + IPoolManager manager, PoolKey memory poolKey, TickCrossingParams memory params ) private returns (PoolParamsOnExecute memory, uint256) { @@ -605,7 +602,7 @@ contract TWAMM is BaseHook, ITWAMM { unchecked { // update pool - (, int128 liquidityNet) = poolManager.getTickLiquidity(poolKey.toId(), params.initializedTick); + (, int128 liquidityNet) = manager.getTickLiquidity(poolKey.toId(), params.initializedTick); if (initializedSqrtPrice < params.pool.sqrtPriceX96) liquidityNet = -liquidityNet; params.pool.liquidity = liquidityNet < 0 ? params.pool.liquidity - uint128(-liquidityNet) @@ -618,7 +615,7 @@ contract TWAMM is BaseHook, ITWAMM { function _isCrossingInitializedTick( PoolParamsOnExecute memory pool, - IPoolManager poolManager, + IPoolManager manager, PoolKey memory poolKey, uint160 nextSqrtPriceX96 ) internal view returns (bool crossingInitializedTick, int24 nextTickInit) { @@ -634,7 +631,7 @@ contract TWAMM is BaseHook, ITWAMM { unchecked { if (searchingLeft) nextTickInit -= 1; } - (nextTickInit, crossingInitializedTick) = poolManager.getNextInitializedTickWithinOneWord( + (nextTickInit, crossingInitializedTick) = manager.getNextInitializedTickWithinOneWord( poolKey.toId(), nextTickInit, poolKey.tickSpacing, searchingLeft ); nextTickInitFurtherThanTarget = searchingLeft ? nextTickInit <= targetTick : nextTickInit > targetTick; diff --git a/contracts/hooks/examples/VolatilityOracle.sol b/contracts/hooks/examples/VolatilityOracle.sol index ede61bf5..1a42e121 100644 --- a/contracts/hooks/examples/VolatilityOracle.sol +++ b/contracts/hooks/examples/VolatilityOracle.sol @@ -19,7 +19,7 @@ contract VolatilityOracle is BaseHook { return uint32(block.timestamp); } - constructor(IPoolManager _poolManager) BaseHook(_poolManager) { + constructor(IPoolManager _manager) BaseHook(_manager) { deployTimestamp = _blockTimestamp(); } @@ -56,7 +56,7 @@ contract VolatilityOracle is BaseHook { uint24 startingFee = 3000; uint32 lapsed = _blockTimestamp() - deployTimestamp; uint24 fee = startingFee + (uint24(lapsed) * 100) / 60; // 100 bps a minute - poolManager.updateDynamicLPFee(key, fee); // initial fee 0.30% + manager.updateDynamicLPFee(key, fee); // initial fee 0.30% } function afterInitialize(address, PoolKey calldata key, uint160, int24, bytes calldata) From af6f768565be008e44b55746d39eac9dc7147a93 Mon Sep 17 00:00:00 2001 From: Junion Date: Tue, 18 Jun 2024 16:10:20 -0400 Subject: [PATCH 10/17] Refactor for generalized FeeTaker abstract contract --- contracts/hooks/examples/FeeTaker.sol | 102 ++++++++++++++++++ contracts/hooks/examples/FeeTaking.sol | 93 +++------------- test/FeeTaking.t.sol | 70 +----------- .../FeeTakingImplementation.sol | 10 +- 4 files changed, 128 insertions(+), 147 deletions(-) create mode 100644 contracts/hooks/examples/FeeTaker.sol diff --git a/contracts/hooks/examples/FeeTaker.sol b/contracts/hooks/examples/FeeTaker.sol new file mode 100644 index 00000000..0921ac79 --- /dev/null +++ b/contracts/hooks/examples/FeeTaker.sol @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import {BaseHook} from "../../BaseHook.sol"; +import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; +import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; +import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; +import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; +import {Currency, CurrencyLibrary} from "@uniswap/v4-core/src/types/Currency.sol"; +import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol"; + +abstract contract FeeTaker is BaseHook { + using SafeCast for uint256; + + bytes internal constant ZERO_BYTES = bytes(""); + + constructor(IPoolManager _poolManager) BaseHook(_poolManager) {} + + /** + * @notice This hook takes a fee from the unspecified token after a swap. + * @dev This can be overridden if more permissions are needed. + */ + function getHookPermissions() public pure virtual override returns (Hooks.Permissions memory) { + return Hooks.Permissions({ + beforeInitialize: false, + afterInitialize: false, + beforeAddLiquidity: false, + afterAddLiquidity: false, + beforeRemoveLiquidity: false, + afterRemoveLiquidity: false, + beforeSwap: false, + afterSwap: true, + beforeDonate: false, + afterDonate: false, + beforeSwapReturnDelta: false, + afterSwapReturnDelta: true, + afterAddLiquidityReturnDelta: false, + afterRemoveLiquidityReturnDelta: false + }); + } + + function afterSwap( + address sender, + PoolKey calldata key, + IPoolManager.SwapParams calldata params, + BalanceDelta delta, + bytes calldata hookData + ) external override onlyByManager returns (bytes4, int128) { + // fee will be in the unspecified token of the swap + bool currency0Specified = (params.amountSpecified < 0 == params.zeroForOne); + (Currency feeCurrency, int128 amountUnspecified) = + (currency0Specified) ? (key.currency1, delta.amount1()) : (key.currency0, delta.amount0()); + // if exactOutput swap, get the absolute output amount + if (amountUnspecified < 0) amountUnspecified = -amountUnspecified; + + uint256 feeAmount = _feeAmount(amountUnspecified); + // mint ERC6909 instead of take to avoid edge case where PM doesn't have enough balance + manager.mint(address(this), CurrencyLibrary.toId(feeCurrency), feeAmount); + + (bytes4 selector, int128 amount) = _afterSwap(sender, key, params, delta, hookData); + return (selector, feeAmount.toInt128() + amount); + } + + function withdraw(Currency[] calldata currencies) external { + manager.unlock(abi.encode(currencies)); + } + + function _unlockCallback(bytes calldata rawData) internal override returns (bytes memory) { + Currency[] memory currencies = abi.decode(rawData, (Currency[])); + uint256 length = currencies.length; + for (uint256 i = 0; i < length;) { + uint256 amount = manager.balanceOf(address(this), CurrencyLibrary.toId(currencies[i])); + manager.burn(address(this), CurrencyLibrary.toId(currencies[i]), amount); + manager.take(currencies[i], _recipient(), amount); + unchecked { + i++; + } + } + return ZERO_BYTES; + } + + /** + * @dev This is a virtual function that should be overridden so it returns the fee charged for a given amount. + */ + function _feeAmount(int128 amountUnspecified) internal view virtual returns (uint256); + + /** + * @dev This is a virtual function that should be overridden so it returns the address to receive the fee. + */ + function _recipient() internal view virtual returns (address); + + /** + * @dev This can be overridden to add logic after a swap. + */ + function _afterSwap(address, PoolKey memory, IPoolManager.SwapParams memory, BalanceDelta, bytes calldata) + internal + virtual + returns (bytes4, int128) + { + return (BaseHook.afterSwap.selector, 0); + } +} diff --git a/contracts/hooks/examples/FeeTaking.sol b/contracts/hooks/examples/FeeTaking.sol index 942f1f3e..ed50e483 100644 --- a/contracts/hooks/examples/FeeTaking.sol +++ b/contracts/hooks/examples/FeeTaking.sol @@ -1,99 +1,36 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.24; -import {BaseHook} from "../../BaseHook.sol"; -import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; -import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; -import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; -import {BeforeSwapDelta, BeforeSwapDeltaLibrary} from "@uniswap/v4-core/src/types/BeforeSwapDelta.sol"; -import {Currency, CurrencyLibrary} from "@uniswap/v4-core/src/types/Currency.sol"; import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol"; import {Owned} from "solmate/auth/Owned.sol"; -import {IUnlockCallback} from "@uniswap/v4-core/src/interfaces/callback/IUnlockCallback.sol"; +import {FeeTaker} from "./FeeTaker.sol"; -contract FeeTaking is BaseHook, IUnlockCallback, Owned { +contract FeeTaking is FeeTaker, Owned { using SafeCast for uint256; - bytes internal constant ZERO_BYTES = bytes(""); uint128 private constant TOTAL_BIPS = 10000; uint128 private constant MAX_BIPS = 100; - uint128 public swapFeeBips; + uint128 public immutable swapFeeBips; + address public treasury; - struct CallbackData { - address to; - Currency[] currencies; - } - - constructor(IPoolManager _poolManager, uint128 _swapFeeBips, address _owner) BaseHook(_poolManager) Owned(_owner) { + constructor(IPoolManager _poolManager, uint128 _swapFeeBips, address _owner, address _treasury) + FeeTaker(_poolManager) + Owned(_owner) + { swapFeeBips = _swapFeeBips; + treasury = _treasury; } - function getHookPermissions() public pure override returns (Hooks.Permissions memory) { - return Hooks.Permissions({ - beforeInitialize: false, - afterInitialize: false, - beforeAddLiquidity: false, - afterAddLiquidity: false, - beforeRemoveLiquidity: false, - afterRemoveLiquidity: false, - beforeSwap: false, - afterSwap: true, - beforeDonate: false, - afterDonate: false, - beforeSwapReturnDelta: false, - afterSwapReturnDelta: true, - afterAddLiquidityReturnDelta: false, - afterRemoveLiquidityReturnDelta: false - }); + function setTreasury(address _treasury) external onlyOwner { + treasury = _treasury; } - function afterSwap( - address, - PoolKey calldata key, - IPoolManager.SwapParams calldata params, - BalanceDelta delta, - bytes calldata - ) external override returns (bytes4, int128) { - // fee will be in the unspecified token of the swap - bool currency0Specified = (params.amountSpecified < 0 == params.zeroForOne); - (Currency feeCurrency, int128 swapAmount) = - (currency0Specified) ? (key.currency1, delta.amount1()) : (key.currency0, delta.amount0()); - // if fee is on output, get the absolute output amount - if (swapAmount < 0) swapAmount = -swapAmount; - - uint256 feeAmount = (uint128(swapAmount) * swapFeeBips) / TOTAL_BIPS; - // mint ERC6909 instead of take to avoid edge case where PM doesn't have enough balance - poolManager.mint(address(this), CurrencyLibrary.toId(feeCurrency), feeAmount); - - return (BaseHook.afterSwap.selector, feeAmount.toInt128()); + function _feeAmount(int128 amountUnspecified) internal view override returns (uint256) { + return uint128(amountUnspecified) * swapFeeBips / TOTAL_BIPS; } - function setSwapFeeBips(uint128 _swapFeeBips) external onlyOwner { - require(_swapFeeBips <= MAX_BIPS); - swapFeeBips = _swapFeeBips; - } - - function withdraw(address to, Currency[] calldata currencies) external onlyOwner { - poolManager.unlock(abi.encode(CallbackData(to, currencies))); - } - - function unlockCallback(bytes calldata rawData) - external - override(IUnlockCallback, BaseHook) - poolManagerOnly - returns (bytes memory) - { - CallbackData memory data = abi.decode(rawData, (CallbackData)); - uint256 length = data.currencies.length; - for (uint256 i = 0; i < length;) { - uint256 amount = poolManager.balanceOf(address(this), CurrencyLibrary.toId(data.currencies[i])); - poolManager.burn(address(this), CurrencyLibrary.toId(data.currencies[i]), amount); - poolManager.take(data.currencies[i], data.to, amount); - unchecked { - i++; - } - } - return ZERO_BYTES; + function _recipient() internal view override returns (address) { + return treasury; } } diff --git a/test/FeeTaking.t.sol b/test/FeeTaking.t.sol index 85de2605..8884bbdf 100644 --- a/test/FeeTaking.t.sol +++ b/test/FeeTaking.t.sol @@ -5,7 +5,6 @@ import {Test} from "forge-std/Test.sol"; import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; import {FeeTaking} from "../contracts/hooks/examples/FeeTaking.sol"; import {FeeTakingImplementation} from "./shared/implementation/FeeTakingImplementation.sol"; -import {PoolManager} from "@uniswap/v4-core/src/PoolManager.sol"; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; import {Deployers} from "@uniswap/v4-core/test/utils/Deployers.sol"; import {TestERC20} from "@uniswap/v4-core/src/test/TestERC20.sol"; @@ -43,7 +42,7 @@ contract FeeTakingTest is Test, Deployers { token1 = TestERC20(Currency.unwrap(currency1)); vm.record(); - FeeTakingImplementation impl = new FeeTakingImplementation(manager, 25, address(this), feeTaking); + FeeTakingImplementation impl = new FeeTakingImplementation(manager, 25, address(this), TREASURY, feeTaking); (, bytes32[] memory writes) = vm.accesses(address(impl)); vm.etch(address(feeTaking), address(impl).code); // for each storage key that was written during the hook implementation, copy the value over @@ -99,7 +98,7 @@ contract FeeTakingTest is Test, Deployers { Currency[] memory currencies = new Currency[](2); currencies[0] = key.currency0; currencies[1] = key.currency1; - feeTaking.withdraw(TREASURY, currencies); + feeTaking.withdraw(currencies); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)), 0); assertEq(currency0.balanceOf(TREASURY) / R, expectedFee2 / R); @@ -148,67 +147,11 @@ contract FeeTakingTest is Test, Deployers { Currency[] memory currencies = new Currency[](2); currencies[0] = key.currency0; currencies[1] = key.currency1; - feeTaking.withdraw(TREASURY, currencies); + feeTaking.withdraw(currencies); assertEq(currency0.balanceOf(TREASURY) / R, 0); assertEq(currency1.balanceOf(TREASURY) / R, (expectedFee + expectedFee2) / R); } - function testSwapWithDifferentFees() public { - testSwapHooks(); - feeTaking.setSwapFeeBips(50); // Set fee to 0.50% - - // Swap exact token0 for token1 // - bool zeroForOne = true; - int256 amountSpecified = -1e12; - BalanceDelta swapDelta = swap(key, zeroForOne, amountSpecified, ZERO_BYTES); - // ---------------------------- // - - uint128 output = uint128(swapDelta.amount1()); - assertTrue(output > 0); - - uint256 expectedFee = calculateFeeForExactInput(output, 50); - - assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); - assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); - } - - function testZeroFeeSwap() public { - feeTaking.setSwapFeeBips(0); // Set fee to 0% - - // Swap exact token0 for token1 // - bool zeroForOne = true; - int256 amountSpecified = -1e12; - BalanceDelta swapDelta = swap(key, zeroForOne, amountSpecified, ZERO_BYTES); - // ---------------------------- // - - uint128 output = uint128(swapDelta.amount1()); - assertTrue(output > 0); - - // No fee should be taken - uint256 expectedFee = calculateFeeForExactInput(output, 0); - assertEq(expectedFee, 0); - assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); - assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)), 0); - } - - function testMaxFeeSwap() public { - feeTaking.setSwapFeeBips(100); // Set fee to 1% - - // Swap exact token0 for token1 // - bool zeroForOne = true; - int256 amountSpecified = -1e12; - BalanceDelta swapDelta = swap(key, zeroForOne, amountSpecified, ZERO_BYTES); - // ---------------------------- // - - uint128 output = uint128(swapDelta.amount1()); - assertTrue(output > 0); - - uint256 expectedFee = calculateFeeForExactInput(output, 100); - - assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); - assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); - } - function testMultiTokenPoolSwap() public { testSwapHooks(); // Deploy additional tokens @@ -243,7 +186,7 @@ contract FeeTakingTest is Test, Deployers { currencies[0] = key.currency0; currencies[1] = key.currency1; currencies[2] = key2.currency1; - feeTaking.withdraw(TREASURY, currencies); + feeTaking.withdraw(currencies); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)), 0); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key2.currency1)), 0); @@ -252,11 +195,6 @@ contract FeeTakingTest is Test, Deployers { assertEq(currency3.balanceOf(TREASURY) / R, expectedFee / R); } - function testTooHighFee() public { - vm.expectRevert(); - feeTaking.setSwapFeeBips(101); - } - function calculateFeeForExactInput(uint256 outputAmount, uint128 feeBips) internal pure returns (uint256) { return outputAmount * TOTAL_BIPS / (TOTAL_BIPS - feeBips) - outputAmount; } diff --git a/test/shared/implementation/FeeTakingImplementation.sol b/test/shared/implementation/FeeTakingImplementation.sol index 697dbf41..a2443123 100644 --- a/test/shared/implementation/FeeTakingImplementation.sol +++ b/test/shared/implementation/FeeTakingImplementation.sol @@ -7,9 +7,13 @@ import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; contract FeeTakingImplementation is FeeTaking { - constructor(IPoolManager _poolManager, uint128 _swapFeeBips, address _owner, FeeTaking addressToEtch) - FeeTaking(_poolManager, _swapFeeBips, _owner) - { + constructor( + IPoolManager _poolManager, + uint128 _swapFeeBips, + address _owner, + address _treasury, + FeeTaking addressToEtch + ) FeeTaking(_poolManager, _swapFeeBips, _owner, _treasury) { Hooks.validateHookPermissions(addressToEtch, getHookPermissions()); } From 3be9116699b15c79e012c300d49ccbedd88e72fa Mon Sep 17 00:00:00 2001 From: Junion Date: Tue, 18 Jun 2024 16:43:17 -0400 Subject: [PATCH 11/17] remove MAX_BIPS --- contracts/hooks/examples/FeeTaking.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/hooks/examples/FeeTaking.sol b/contracts/hooks/examples/FeeTaking.sol index ed50e483..c0099149 100644 --- a/contracts/hooks/examples/FeeTaking.sol +++ b/contracts/hooks/examples/FeeTaking.sol @@ -10,7 +10,6 @@ contract FeeTaking is FeeTaker, Owned { using SafeCast for uint256; uint128 private constant TOTAL_BIPS = 10000; - uint128 private constant MAX_BIPS = 100; uint128 public immutable swapFeeBips; address public treasury; From bd34d18e8bbdcaf69177e9778bfa9c3a3eddb58a Mon Sep 17 00:00:00 2001 From: Junion Date: Tue, 18 Jun 2024 18:23:14 -0400 Subject: [PATCH 12/17] preemptively use library --- contracts/hooks/examples/FeeTaker.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contracts/hooks/examples/FeeTaker.sol b/contracts/hooks/examples/FeeTaker.sol index 0921ac79..5563a113 100644 --- a/contracts/hooks/examples/FeeTaker.sol +++ b/contracts/hooks/examples/FeeTaker.sol @@ -46,16 +46,18 @@ abstract contract FeeTaker is BaseHook { BalanceDelta delta, bytes calldata hookData ) external override onlyByManager returns (bytes4, int128) { + //(Currency currencyUnspecified, amountUnspecified) = key.getUnspecified(params); + // fee will be in the unspecified token of the swap bool currency0Specified = (params.amountSpecified < 0 == params.zeroForOne); - (Currency feeCurrency, int128 amountUnspecified) = + (Currency currencyUnspecified, int128 amountUnspecified) = (currency0Specified) ? (key.currency1, delta.amount1()) : (key.currency0, delta.amount0()); // if exactOutput swap, get the absolute output amount if (amountUnspecified < 0) amountUnspecified = -amountUnspecified; uint256 feeAmount = _feeAmount(amountUnspecified); // mint ERC6909 instead of take to avoid edge case where PM doesn't have enough balance - manager.mint(address(this), CurrencyLibrary.toId(feeCurrency), feeAmount); + manager.mint(address(this), CurrencyLibrary.toId(currencyUnspecified), feeAmount); (bytes4 selector, int128 amount) = _afterSwap(sender, key, params, delta, hookData); return (selector, feeAmount.toInt128() + amount); From c1ccd70b0d4492202475a7bf8069dce755950458 Mon Sep 17 00:00:00 2001 From: Junion Date: Fri, 21 Jun 2024 11:07:24 -0400 Subject: [PATCH 13/17] update README --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index b3355a10..731bd92a 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,9 @@ If you’re interested in contributing please see the [contribution guidelines]( contracts/ ----hooks/ ----examples/ + | FeeTaker.sol + | FeeTaking.sol + | FullRange.sol | GeomeanOracle.sol | LimitOrder.sol | TWAMM.sol From 91a518587e3b5e8e1362c8a32e8e412a400b1515 Mon Sep 17 00:00:00 2001 From: Junion Date: Fri, 21 Jun 2024 11:17:37 -0400 Subject: [PATCH 14/17] add snapshots --- .forge-snapshots/FeeTakingFirstSwap.snap | 1 + .forge-snapshots/FeeTakingSecondSwap.snap | 1 + .forge-snapshots/FeeTakingWithdrawTwoTokens.snap | 1 + test/FeeTaking.t.sol | 11 ++++++++--- 4 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 .forge-snapshots/FeeTakingFirstSwap.snap create mode 100644 .forge-snapshots/FeeTakingSecondSwap.snap create mode 100644 .forge-snapshots/FeeTakingWithdrawTwoTokens.snap diff --git a/.forge-snapshots/FeeTakingFirstSwap.snap b/.forge-snapshots/FeeTakingFirstSwap.snap new file mode 100644 index 00000000..51d070ee --- /dev/null +++ b/.forge-snapshots/FeeTakingFirstSwap.snap @@ -0,0 +1 @@ +162044 \ No newline at end of file diff --git a/.forge-snapshots/FeeTakingSecondSwap.snap b/.forge-snapshots/FeeTakingSecondSwap.snap new file mode 100644 index 00000000..74b56108 --- /dev/null +++ b/.forge-snapshots/FeeTakingSecondSwap.snap @@ -0,0 +1 @@ +86285 \ No newline at end of file diff --git a/.forge-snapshots/FeeTakingWithdrawTwoTokens.snap b/.forge-snapshots/FeeTakingWithdrawTwoTokens.snap new file mode 100644 index 00000000..97b53e53 --- /dev/null +++ b/.forge-snapshots/FeeTakingWithdrawTwoTokens.snap @@ -0,0 +1 @@ +71449 \ No newline at end of file diff --git a/test/FeeTaking.t.sol b/test/FeeTaking.t.sol index 8884bbdf..9bbd0c34 100644 --- a/test/FeeTaking.t.sol +++ b/test/FeeTaking.t.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.19; import {Test} from "forge-std/Test.sol"; +import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol"; import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; import {FeeTaking} from "../contracts/hooks/examples/FeeTaking.sol"; import {FeeTakingImplementation} from "./shared/implementation/FeeTakingImplementation.sol"; @@ -15,7 +16,7 @@ import {HookEnabledSwapRouter} from "./utils/HookEnabledSwapRouter.sol"; import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; -contract FeeTakingTest is Test, Deployers { +contract FeeTakingTest is Test, Deployers, GasSnapshot { using PoolIdLibrary for PoolKey; using StateLibrary for IPoolManager; @@ -66,11 +67,12 @@ contract FeeTakingTest is Test, Deployers { assertEq(currency0.balanceOf(TREASURY), 0); assertEq(currency1.balanceOf(TREASURY), 0); + snapStart("FeeTakingFirstSwap"); // Swap exact token0 for token1 // bool zeroForOne = true; int256 amountSpecified = -1e12; BalanceDelta swapDelta = swap(key, zeroForOne, amountSpecified, ZERO_BYTES); - // ---------------------------- // + snapEnd(); uint128 output = uint128(swapDelta.amount1()); assertTrue(output > 0); @@ -80,11 +82,12 @@ contract FeeTakingTest is Test, Deployers { assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R); + snapStart("FeeTakingSecondSwap"); // Swap token0 for exact token1 // bool zeroForOne2 = true; int256 amountSpecified2 = 1e12; // positive number indicates exact output swap BalanceDelta swapDelta2 = swap(key, zeroForOne2, amountSpecified2, ZERO_BYTES); - // ---------------------------- // + snapEnd(); uint128 input = uint128(-swapDelta2.amount0()); assertTrue(output > 0); @@ -98,7 +101,9 @@ contract FeeTakingTest is Test, Deployers { Currency[] memory currencies = new Currency[](2); currencies[0] = key.currency0; currencies[1] = key.currency1; + snapStart("FeeTakingWithdrawTwoTokens"); feeTaking.withdraw(currencies); + snapEnd(); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency0)), 0); assertEq(manager.balanceOf(address(feeTaking), CurrencyLibrary.toId(key.currency1)), 0); assertEq(currency0.balanceOf(TREASURY) / R, expectedFee2 / R); From 173b3ac1f98f11ba09e2d38f4e2cd780266799ce Mon Sep 17 00:00:00 2001 From: Junion <69495294+Jun1on@users.noreply.github.com> Date: Fri, 21 Jun 2024 14:48:48 -0400 Subject: [PATCH 15/17] gas optimization --- .forge-snapshots/FeeTakingWithdrawTwoTokens.snap | 2 +- contracts/hooks/examples/FeeTaker.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.forge-snapshots/FeeTakingWithdrawTwoTokens.snap b/.forge-snapshots/FeeTakingWithdrawTwoTokens.snap index 97b53e53..6565a566 100644 --- a/.forge-snapshots/FeeTakingWithdrawTwoTokens.snap +++ b/.forge-snapshots/FeeTakingWithdrawTwoTokens.snap @@ -1 +1 @@ -71449 \ No newline at end of file +71441 \ No newline at end of file diff --git a/contracts/hooks/examples/FeeTaker.sol b/contracts/hooks/examples/FeeTaker.sol index 5563a113..91d1fb7d 100644 --- a/contracts/hooks/examples/FeeTaker.sol +++ b/contracts/hooks/examples/FeeTaker.sol @@ -75,7 +75,7 @@ abstract contract FeeTaker is BaseHook { manager.burn(address(this), CurrencyLibrary.toId(currencies[i]), amount); manager.take(currencies[i], _recipient(), amount); unchecked { - i++; + ++i; } } return ZERO_BYTES; From 35ba973e854ba08c3167bba34bcd2a3d20e6da74 Mon Sep 17 00:00:00 2001 From: Junion <69495294+Jun1on@users.noreply.github.com> Date: Wed, 3 Jul 2024 15:48:07 -0400 Subject: [PATCH 16/17] test _afterSwap extension --- .forge-snapshots/FeeTakingFirstSwap.snap | 2 +- .forge-snapshots/FeeTakingSecondSwap.snap | 2 +- .../FeeTakingWithdrawTwoTokens.snap | 2 +- contracts/hooks/examples/FeeTaker.sol | 2 +- contracts/hooks/examples/FeeTaking.sol | 2 +- lib/forge-gas-snapshot | 2 +- lib/forge-std | 2 +- test/FeeTaking.t.sol | 107 ++++++++++++++++-- .../implementation/FeeTakingExtension.sol | 61 ++++++++++ 9 files changed, 168 insertions(+), 14 deletions(-) create mode 100644 test/shared/implementation/FeeTakingExtension.sol diff --git a/.forge-snapshots/FeeTakingFirstSwap.snap b/.forge-snapshots/FeeTakingFirstSwap.snap index 51d070ee..ae8cb8b8 100644 --- a/.forge-snapshots/FeeTakingFirstSwap.snap +++ b/.forge-snapshots/FeeTakingFirstSwap.snap @@ -1 +1 @@ -162044 \ No newline at end of file +112467 \ No newline at end of file diff --git a/.forge-snapshots/FeeTakingSecondSwap.snap b/.forge-snapshots/FeeTakingSecondSwap.snap index 74b56108..d18f8244 100644 --- a/.forge-snapshots/FeeTakingSecondSwap.snap +++ b/.forge-snapshots/FeeTakingSecondSwap.snap @@ -1 +1 @@ -86285 \ No newline at end of file +86214 \ No newline at end of file diff --git a/.forge-snapshots/FeeTakingWithdrawTwoTokens.snap b/.forge-snapshots/FeeTakingWithdrawTwoTokens.snap index 6565a566..98af7300 100644 --- a/.forge-snapshots/FeeTakingWithdrawTwoTokens.snap +++ b/.forge-snapshots/FeeTakingWithdrawTwoTokens.snap @@ -1 +1 @@ -71441 \ No newline at end of file +69428 \ No newline at end of file diff --git a/contracts/hooks/examples/FeeTaker.sol b/contracts/hooks/examples/FeeTaker.sol index 91d1fb7d..a58a9631 100644 --- a/contracts/hooks/examples/FeeTaker.sol +++ b/contracts/hooks/examples/FeeTaker.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: MIT +// SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.24; import {BaseHook} from "../../BaseHook.sol"; diff --git a/contracts/hooks/examples/FeeTaking.sol b/contracts/hooks/examples/FeeTaking.sol index c0099149..23ead10c 100644 --- a/contracts/hooks/examples/FeeTaking.sol +++ b/contracts/hooks/examples/FeeTaking.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: MIT +// SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.24; import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; diff --git a/lib/forge-gas-snapshot b/lib/forge-gas-snapshot index 2f884282..9161f7c0 160000 --- a/lib/forge-gas-snapshot +++ b/lib/forge-gas-snapshot @@ -1 +1 @@ -Subproject commit 2f884282b4cd067298e798974f5b534288b13bc2 +Subproject commit 9161f7c0b6c6788a89081e2b3b9c67592b71e689 diff --git a/lib/forge-std b/lib/forge-std index 2b58ecbc..75b3fcf0 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 2b58ecbcf3dfde7a75959dc7b4eb3d0670278de6 +Subproject commit 75b3fcf052cc7886327e4c2eac3d1a1f36942b41 diff --git a/test/FeeTaking.t.sol b/test/FeeTaking.t.sol index 9bbd0c34..6612380b 100644 --- a/test/FeeTaking.t.sol +++ b/test/FeeTaking.t.sol @@ -15,6 +15,7 @@ import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; import {HookEnabledSwapRouter} from "./utils/HookEnabledSwapRouter.sol"; import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol"; import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; +import {FeeTakingExtension} from "./shared/implementation/FeeTakingExtension.sol"; contract FeeTakingTest is Test, Deployers, GasSnapshot { using PoolIdLibrary for PoolKey; @@ -32,9 +33,11 @@ contract FeeTakingTest is Test, Deployers, GasSnapshot { TestERC20 token0; TestERC20 token1; FeeTaking feeTaking = FeeTaking(address(uint160(Hooks.AFTER_SWAP_FLAG | Hooks.AFTER_SWAP_RETURNS_DELTA_FLAG))); + FeeTakingExtension feeTakingExtension = + FeeTakingExtension(address(0x100000 | uint160(Hooks.AFTER_SWAP_FLAG | Hooks.AFTER_SWAP_RETURNS_DELTA_FLAG))); PoolId id; - function setUp() public { + function setUpNormal() public { deployFreshManagerAndRouters(); (currency0, currency1) = deployMintAndApprove2Currencies(); @@ -46,24 +49,46 @@ contract FeeTakingTest is Test, Deployers, GasSnapshot { FeeTakingImplementation impl = new FeeTakingImplementation(manager, 25, address(this), TREASURY, feeTaking); (, bytes32[] memory writes) = vm.accesses(address(impl)); vm.etch(address(feeTaking), address(impl).code); - // for each storage key that was written during the hook implementation, copy the value over unchecked { for (uint256 i = 0; i < writes.length; i++) { bytes32 slot = writes[i]; vm.store(address(feeTaking), slot, vm.load(address(impl), slot)); } } - - // key = PoolKey(currency0, currency1, 3000, 60, feeTaking); (key, id) = initPoolAndAddLiquidity(currency0, currency1, feeTaking, 3000, SQRT_PRICE_1_1, ZERO_BYTES); - token0.approve(address(feeTaking), type(uint256).max); - token1.approve(address(feeTaking), type(uint256).max); token0.approve(address(router), type(uint256).max); token1.approve(address(router), type(uint256).max); } + function setUpExtension() public { + deployFreshManagerAndRouters(); + (currency0, currency1) = deployMintAndApprove2Currencies(); + + router = new HookEnabledSwapRouter(manager); + token0 = TestERC20(Currency.unwrap(currency0)); + token1 = TestERC20(Currency.unwrap(currency1)); + + vm.record(); + FeeTakingExtension impl = new FeeTakingExtension(manager, 25, address(this), TREASURY); + (, bytes32[] memory writes) = vm.accesses(address(impl)); + vm.etch(address(feeTakingExtension), address(impl).code); + unchecked { + for (uint256 i = 0; i < writes.length; i++) { + bytes32 slot = writes[i]; + vm.store(address(feeTakingExtension), slot, vm.load(address(impl), slot)); + } + } + (key, id) = initPoolAndAddLiquidity(currency0, currency1, feeTakingExtension, 3000, SQRT_PRICE_1_1, ZERO_BYTES); + + token0.approve(address(router), type(uint256).max); + token1.approve(address(router), type(uint256).max); + token0.transfer(address(feeTakingExtension), 1e18); + token1.transfer(address(feeTakingExtension), 1e18); + } + function testSwapHooks() public { + setUpNormal(); assertEq(currency0.balanceOf(TREASURY), 0); assertEq(currency1.balanceOf(TREASURY), 0); @@ -90,7 +115,7 @@ contract FeeTakingTest is Test, Deployers, GasSnapshot { snapEnd(); uint128 input = uint128(-swapDelta2.amount0()); - assertTrue(output > 0); + assertTrue(input > 0); uint256 expectedFee2 = calculateFeeForExactOutput(input, feeTaking.swapFeeBips()); @@ -112,6 +137,7 @@ contract FeeTakingTest is Test, Deployers, GasSnapshot { // this would error had the hook not used ERC6909 function testEdgeCase() public { + setUpNormal(); // first, deplete the pool of token1 // Swap exact token0 for token1 // bool zeroForOne = true; @@ -200,6 +226,73 @@ contract FeeTakingTest is Test, Deployers, GasSnapshot { assertEq(currency3.balanceOf(TREASURY) / R, expectedFee / R); } + function testHookExtension() public { + setUpExtension(); + assertEq(currency0.balanceOf(TREASURY), 0); + assertEq(currency1.balanceOf(TREASURY), 0); + + // Swap exact token0 for token1 // + bool zeroForOne = true; + int256 amountSpecified = -1e12; + BalanceDelta swapDelta = swap(key, zeroForOne, amountSpecified, ZERO_BYTES); + + assertEq(feeTakingExtension.afterSwapCounter(), 1); + + uint128 output = uint128(swapDelta.amount1() - feeTakingExtension.DONATION_AMOUNT()); + assertTrue(output > 0); + + uint256 expectedFee = calculateFeeForExactInput(output, feeTakingExtension.swapFeeBips()); + + assertEq(manager.balanceOf(address(feeTakingExtension), CurrencyLibrary.toId(key.currency0)), 0); + assertEq( + manager.balanceOf(address(feeTakingExtension), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R + ); + + assertEq(currency0.balanceOf(address(feeTakingExtension)), 1 ether); + assertEq( + currency1.balanceOf(address(feeTakingExtension)), + 1 ether - uint256(int256(feeTakingExtension.DONATION_AMOUNT())) + ); + + // Swap token0 for exact token1 // + bool zeroForOne2 = true; + int256 amountSpecified2 = 1e12; // positive number indicates exact output swap + BalanceDelta swapDelta2 = swap(key, zeroForOne2, amountSpecified2, ZERO_BYTES); + return; + assertEq(feeTakingExtension.afterSwapCounter(), 2); + + uint128 input = uint128(-swapDelta2.amount0() + feeTakingExtension.DONATION_AMOUNT()); + assertTrue(input > 0); + + uint256 expectedFee2 = calculateFeeForExactOutput(input, feeTakingExtension.swapFeeBips()); + + assertEq( + manager.balanceOf(address(feeTakingExtension), CurrencyLibrary.toId(key.currency0)) / R, expectedFee2 / R + ); + assertEq( + manager.balanceOf(address(feeTakingExtension), CurrencyLibrary.toId(key.currency1)) / R, expectedFee / R + ); + + assertEq( + currency0.balanceOf(address(feeTakingExtension)), + 1 ether - uint256(int256(feeTakingExtension.DONATION_AMOUNT())) + ); + assertEq( + currency1.balanceOf(address(feeTakingExtension)), + 1 ether - uint256(int256(feeTakingExtension.DONATION_AMOUNT())) + ); + + // test withdrawing tokens // + Currency[] memory currencies = new Currency[](2); + currencies[0] = key.currency0; + currencies[1] = key.currency1; + feeTakingExtension.withdraw(currencies); + assertEq(manager.balanceOf(address(feeTakingExtension), CurrencyLibrary.toId(key.currency0)), 0); + assertEq(manager.balanceOf(address(feeTakingExtension), CurrencyLibrary.toId(key.currency1)), 0); + assertEq(currency0.balanceOf(TREASURY) / R, expectedFee2 / R); + assertEq(currency1.balanceOf(TREASURY) / R, expectedFee / R); + } + function calculateFeeForExactInput(uint256 outputAmount, uint128 feeBips) internal pure returns (uint256) { return outputAmount * TOTAL_BIPS / (TOTAL_BIPS - feeBips) - outputAmount; } diff --git a/test/shared/implementation/FeeTakingExtension.sol b/test/shared/implementation/FeeTakingExtension.sol new file mode 100644 index 00000000..70d0f3b7 --- /dev/null +++ b/test/shared/implementation/FeeTakingExtension.sol @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.24; + +import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; +import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol"; +import {Owned} from "solmate/auth/Owned.sol"; +import {FeeTaker} from "./../../../contracts/hooks/examples/FeeTaker.sol"; +import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol"; +import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; +import {BaseHook} from "./../../../contracts/BaseHook.sol"; +import {Currency, CurrencyLibrary} from "@uniswap/v4-core/src/types/Currency.sol"; + +contract FeeTakingExtension is FeeTaker, Owned { + using SafeCast for uint256; + using CurrencyLibrary for Currency; + + uint128 private constant TOTAL_BIPS = 10000; + uint128 public immutable swapFeeBips; + address public treasury; + + uint256 public afterSwapCounter; + int128 public DONATION_AMOUNT = 1 gwei; + + constructor(IPoolManager _poolManager, uint128 _swapFeeBips, address _owner, address _treasury) + FeeTaker(_poolManager) + Owned(_owner) + { + swapFeeBips = _swapFeeBips; + treasury = _treasury; + } + + function _afterSwap( + address, + PoolKey memory key, + IPoolManager.SwapParams memory params, + BalanceDelta, + bytes calldata + ) internal override returns (bytes4, int128) { + afterSwapCounter++; + bool currency0Specified = (params.amountSpecified < 0 == params.zeroForOne); + Currency currencyUnspecified = currency0Specified ? key.currency1 : key.currency0; + currencyUnspecified.transfer(address(manager), uint256(int256(DONATION_AMOUNT))); + manager.settle(currencyUnspecified); + return (BaseHook.afterSwap.selector, -DONATION_AMOUNT); + } + + function setTreasury(address _treasury) external onlyOwner { + treasury = _treasury; + } + + function _feeAmount(int128 amountUnspecified) internal view override returns (uint256) { + return uint128(amountUnspecified) * swapFeeBips / TOTAL_BIPS; + } + + function _recipient() internal view override returns (address) { + return treasury; + } + + // make this a no-op in testing + function validateHookAddress(BaseHook _this) internal pure override {} +} From cbaa07b2b9203801569630d62cf42e5a68751b7a Mon Sep 17 00:00:00 2001 From: Junion <69495294+Jun1on@users.noreply.github.com> Date: Tue, 16 Jul 2024 14:04:42 -0400 Subject: [PATCH 17/17] increase foundry gas_limit --- .github/workflows/lint.yml | 2 +- foundry.toml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 9d16a66b..b0eb346e 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -21,5 +21,5 @@ jobs: with: version: nightly - - name: Run tests + - name: Check format run: forge fmt --check diff --git a/foundry.toml b/foundry.toml index 4e95a213..f9da8541 100644 --- a/foundry.toml +++ b/foundry.toml @@ -6,6 +6,7 @@ optimizer_runs = 1000000 ffi = true fs_permissions = [{ access = "read-write", path = ".forge-snapshots/"}] evm_version = "cancun" +gas_limit = "3000000000" [profile.ci] fuzz_runs = 100000