From 7f062b2dc2f786a10c67e831fb35fbe334241208 Mon Sep 17 00:00:00 2001 From: engn33r Date: Mon, 13 May 2024 13:58:44 +0000 Subject: [PATCH 1/9] Fix typo in PoolManager.sol (#644) --- src/PoolManager.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PoolManager.sol b/src/PoolManager.sol index 8b01de5bd..413006c23 100644 --- a/src/PoolManager.sol +++ b/src/PoolManager.sol @@ -174,7 +174,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim key.hooks.afterInitialize(key, sqrtPriceX96, tick, hookData); - // On intitalize we emit the key's fee, which tells us all fee settings a pool can have: either a static swap fee or dynamic swap fee and if the hook has enabled swap or withdraw fees. + // On initialize we emit the key's fee, which tells us all fee settings a pool can have: either a static swap fee or dynamic swap fee and if the hook has enabled swap or withdraw fees. emit Initialize(id, key.currency0, key.currency1, key.fee, key.tickSpacing, key.hooks); } From 2ca3f3beec4b11640ec5061bbb72f671d58e039c Mon Sep 17 00:00:00 2001 From: Alice <34962750+hensha256@users.noreply.github.com> Date: Mon, 13 May 2024 13:25:42 -0300 Subject: [PATCH 2/9] Accurate swap gas (#645) * Update tests to use lastCallGas * Update docs and ci * snapshots revert in CI if wrong * accurate swap gas * include old tests --- .forge-snapshots/simple swap with native.snap | 2 +- .forge-snapshots/simple swap.snap | 2 +- src/test/PoolSwapTest.sol | 2 - src/test/SwapRouterNoChecks.sol | 47 +++++++++++++++++++ test/PoolManager.t.sol | 18 ++++++- test/utils/Deployers.sol | 6 ++- 6 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 src/test/SwapRouterNoChecks.sol diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap index eca48267a..6dae873bb 100644 --- a/.forge-snapshots/simple swap with native.snap +++ b/.forge-snapshots/simple swap with native.snap @@ -1 +1 @@ -131040 \ No newline at end of file +117440 \ No newline at end of file diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap index 095efb63e..7652dce80 100644 --- a/.forge-snapshots/simple swap.snap +++ b/.forge-snapshots/simple swap.snap @@ -1 +1 @@ -149399 \ No newline at end of file +132636 \ No newline at end of file diff --git a/src/test/PoolSwapTest.sol b/src/test/PoolSwapTest.sol index 5bb773e8b..b1bdd5b24 100644 --- a/src/test/PoolSwapTest.sol +++ b/src/test/PoolSwapTest.sol @@ -8,8 +8,6 @@ import {PoolKey} from "../types/PoolKey.sol"; import {IHooks} from "../interfaces/IHooks.sol"; import {Hooks} from "../libraries/Hooks.sol"; import {PoolTestBase} from "./PoolTestBase.sol"; -import {Hooks} from "../libraries/Hooks.sol"; -import {IHooks} from "../interfaces/IHooks.sol"; import {CurrencySettleTake} from "../libraries/CurrencySettleTake.sol"; contract PoolSwapTest is PoolTestBase { diff --git a/src/test/SwapRouterNoChecks.sol b/src/test/SwapRouterNoChecks.sol new file mode 100644 index 000000000..9c00976d8 --- /dev/null +++ b/src/test/SwapRouterNoChecks.sol @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.24; + +import {CurrencyLibrary, Currency} from "../types/Currency.sol"; +import {IPoolManager} from "../interfaces/IPoolManager.sol"; +import {BalanceDelta, BalanceDeltaLibrary} from "../types/BalanceDelta.sol"; +import {PoolKey} from "../types/PoolKey.sol"; +import {IHooks} from "../interfaces/IHooks.sol"; +import {Hooks} from "../libraries/Hooks.sol"; +import {PoolTestBase} from "./PoolTestBase.sol"; +import {CurrencySettleTake} from "../libraries/CurrencySettleTake.sol"; + +contract SwapRouterNoChecks is PoolTestBase { + using CurrencyLibrary for Currency; + using CurrencySettleTake for Currency; + using Hooks for IHooks; + + constructor(IPoolManager _manager) PoolTestBase(_manager) {} + + error NoSwapOccurred(); + + struct CallbackData { + address sender; + PoolKey key; + IPoolManager.SwapParams params; + } + + function swap(PoolKey memory key, IPoolManager.SwapParams memory params) external payable { + manager.unlock(abi.encode(CallbackData(msg.sender, key, params))); + } + + function unlockCallback(bytes calldata rawData) external returns (bytes memory) { + require(msg.sender == address(manager)); + + CallbackData memory data = abi.decode(rawData, (CallbackData)); + + BalanceDelta delta = manager.swap(data.key, data.params, new bytes(0)); + + if (data.params.zeroForOne) { + data.key.currency0.settle(manager, data.sender, uint256(int256(-delta.amount0())), false); + data.key.currency1.take(manager, data.sender, uint256(int256(delta.amount1())), false); + } else { + data.key.currency1.settle(manager, data.sender, uint256(int256(-delta.amount1())), false); + data.key.currency0.take(manager, data.sender, uint256(int256(delta.amount0())), false); + } + } +} diff --git a/test/PoolManager.t.sol b/test/PoolManager.t.sol index 8df6ddbcf..75aff2973 100644 --- a/test/PoolManager.t.sol +++ b/test/PoolManager.t.sol @@ -638,7 +638,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { swapRouter.swap(key, swapParams, testSettings, ZERO_BYTES); } - function test_swap_gas() public { + function test_swap_succeeds() public { IPoolManager.SwapParams memory swapParams = IPoolManager.SwapParams({zeroForOne: true, amountSpecified: -100, sqrtPriceLimitX96: SQRT_PRICE_1_2}); @@ -646,10 +646,17 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false}); swapRouter.swap(key, swapParams, testSettings, ZERO_BYTES); + } + + function test_swap_gas() public { + IPoolManager.SwapParams memory swapParams = + IPoolManager.SwapParams({zeroForOne: true, amountSpecified: -100, sqrtPriceLimitX96: SQRT_PRICE_1_2}); + + swapRouterNoChecks.swap(key, swapParams); snapLastCall("simple swap"); } - function test_swap_withNative_gas() public { + function test_swap_withNative_succeeds() public { IPoolManager.SwapParams memory swapParams = IPoolManager.SwapParams({zeroForOne: true, amountSpecified: -100, sqrtPriceLimitX96: SQRT_PRICE_1_2}); @@ -657,6 +664,13 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false}); swapRouter.swap{value: 100}(nativeKey, swapParams, testSettings, ZERO_BYTES); + } + + function test_swap_withNative_gas() public { + IPoolManager.SwapParams memory swapParams = + IPoolManager.SwapParams({zeroForOne: true, amountSpecified: -100, sqrtPriceLimitX96: SQRT_PRICE_1_2}); + + swapRouterNoChecks.swap{value: 100}(nativeKey, swapParams); snapLastCall("simple swap with native"); } diff --git a/test/utils/Deployers.sol b/test/utils/Deployers.sol index 17b8c7b21..9a0df5ed4 100644 --- a/test/utils/Deployers.sol +++ b/test/utils/Deployers.sol @@ -16,6 +16,7 @@ import {Constants} from "../utils/Constants.sol"; import {SortTokens} from "./SortTokens.sol"; import {PoolModifyLiquidityTest} from "../../src/test/PoolModifyLiquidityTest.sol"; import {PoolSwapTest} from "../../src/test/PoolSwapTest.sol"; +import {SwapRouterNoChecks} from "../../src/test/SwapRouterNoChecks.sol"; import {PoolDonateTest} from "../../src/test/PoolDonateTest.sol"; import {PoolNestedActionsTest} from "../../src/test/PoolNestedActionsTest.sol"; import {PoolTakeTest} from "../../src/test/PoolTakeTest.sol"; @@ -55,6 +56,7 @@ contract Deployers { Currency internal currency1; IPoolManager manager; PoolModifyLiquidityTest modifyLiquidityRouter; + SwapRouterNoChecks swapRouterNoChecks; PoolSwapTest swapRouter; PoolDonateTest donateRouter; PoolTakeTest takeRouter; @@ -93,6 +95,7 @@ contract Deployers { function deployFreshManagerAndRouters() internal { deployFreshManager(); swapRouter = new PoolSwapTest(manager); + swapRouterNoChecks = new SwapRouterNoChecks(manager); modifyLiquidityRouter = new PoolModifyLiquidityTest(manager); donateRouter = new PoolDonateTest(manager); takeRouter = new PoolTakeTest(manager); @@ -122,8 +125,9 @@ contract Deployers { function deployMintAndApproveCurrency() internal returns (Currency currency) { MockERC20 token = deployTokens(1, 2 ** 255)[0]; - address[6] memory toApprove = [ + address[7] memory toApprove = [ address(swapRouter), + address(swapRouterNoChecks), address(modifyLiquidityRouter), address(donateRouter), address(takeRouter), From 8df7bcf30dab9060bf829b749c891c0f343a1239 Mon Sep 17 00:00:00 2001 From: Sara Reynolds <30504811+snreynolds@users.noreply.github.com> Date: Mon, 13 May 2024 14:46:29 -0400 Subject: [PATCH 3/9] clean up delegatecall unit tests (#545) * clean up delegatecall unit tests * comments * relative imports, refresh gas snaps * new name --- ...eCallOverhead.snap => NoDelegateCall.snap} | 0 test/NoDelegateCall.t.sol | 19 +++++++++---------- 2 files changed, 9 insertions(+), 10 deletions(-) rename .forge-snapshots/{NoDelegateCallOverhead.snap => NoDelegateCall.snap} (100%) diff --git a/.forge-snapshots/NoDelegateCallOverhead.snap b/.forge-snapshots/NoDelegateCall.snap similarity index 100% rename from .forge-snapshots/NoDelegateCallOverhead.snap rename to .forge-snapshots/NoDelegateCall.snap diff --git a/test/NoDelegateCall.t.sol b/test/NoDelegateCall.t.sol index b6f688476..4b865841c 100644 --- a/test/NoDelegateCall.t.sol +++ b/test/NoDelegateCall.t.sol @@ -4,44 +4,43 @@ pragma solidity ^0.8.20; import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol"; import {Test} from "forge-std/Test.sol"; import {NoDelegateCallTest} from "../src/test/NoDelegateCallTest.sol"; +import {NoDelegateCall} from "../src/NoDelegateCall.sol"; contract TestDelegateCall is Test, GasSnapshot { - error DelegateCallNotAllowed(); - NoDelegateCallTest noDelegateCallTest; function setUp() public { noDelegateCallTest = new NoDelegateCallTest(); } - function testGasOverhead() public { + function test_gas_noDelegateCall() public { snap( - "NoDelegateCallOverhead", + "NoDelegateCall", noDelegateCallTest.getGasCostOfCannotBeDelegateCalled() - noDelegateCallTest.getGasCostOfCanBeDelegateCalled() ); } - function testDelegateCallNoModifier() public { + function test_delegateCallNoModifier() public { (bool success,) = address(noDelegateCallTest).delegatecall(abi.encode(noDelegateCallTest.canBeDelegateCalled.selector)); assertTrue(success); } - function testDelegateCallWithModifier() public { - vm.expectRevert(DelegateCallNotAllowed.selector); + function test_delegateCallWithModifier_revertsWithDelegateCallNotAllowed() public { + vm.expectRevert(NoDelegateCall.DelegateCallNotAllowed.selector); (bool success,) = address(noDelegateCallTest).delegatecall(abi.encode(noDelegateCallTest.cannotBeDelegateCalled.selector)); // note vm.expectRevert inverts success, so a true result here means it reverted assertTrue(success); } - function testCanCallIntoPrivateMethodWithModifier() public view { + function test_externalCallToPrivateMethodWithModifer_succeeds() public view { noDelegateCallTest.callsIntoNoDelegateCallFunction(); } - function testCannotDelegateCallPrivateMethodWithModifier() public { - vm.expectRevert(DelegateCallNotAllowed.selector); + function test_delegateCallFromExternalToPrivateMethodWithModifier_revertsWithDelegateCallNotAllowed() public { + vm.expectRevert(NoDelegateCall.DelegateCallNotAllowed.selector); (bool success,) = address(noDelegateCallTest).delegatecall( abi.encode(noDelegateCallTest.callsIntoNoDelegateCallFunction.selector) ); From c522ed01e9f20de862f5a788c4c6a7188d788104 Mon Sep 17 00:00:00 2001 From: Daniel Gretzke Date: Tue, 14 May 2024 00:08:28 +0200 Subject: [PATCH 4/9] Only require tests to pass before merging into main or dev branch (#646) --- .github/workflows/tests-pr.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/tests-pr.yml b/.github/workflows/tests-pr.yml index d06191042..5eb80cec9 100644 --- a/.github/workflows/tests-pr.yml +++ b/.github/workflows/tests-pr.yml @@ -2,6 +2,9 @@ name: Tests on: pull_request: + branches: + - main + - dev jobs: forge-tests: From 30671fdcc0060dce1d0e5511e37e99cc77bfcea7 Mon Sep 17 00:00:00 2001 From: Alice <34962750+hensha256@users.noreply.github.com> Date: Mon, 13 May 2024 19:29:07 -0300 Subject: [PATCH 5/9] Before swap returns unspecified delta (#642) * refactor types * Remove HookReturnDelta type * PR comments * linting --- .../poolManager bytecode size.snap | 2 +- .forge-snapshots/simple swap with native.snap | 2 +- .forge-snapshots/simple swap.snap | 2 +- .../swap CA custom curve + swap noop.snap | 2 +- .../swap CA fee on unspecified.snap | 2 +- ...p against liquidity with native token.snap | 2 +- .forge-snapshots/swap against liquidity.snap | 2 +- .../swap burn 6909 for input.snap | 2 +- .../swap burn native 6909 for input.snap | 2 +- .../swap mint native output as 6909.snap | 2 +- .../swap mint output as 6909.snap | 2 +- ...wap skips hook call if hook is caller.snap | 2 +- .forge-snapshots/swap with dynamic fee.snap | 2 +- .forge-snapshots/swap with hooks.snap | 2 +- .../swap with lp fee and protocol fee.snap | 2 +- .../update dynamic fee in before swap.snap | 2 +- src/PoolManager.sol | 5 +- src/interfaces/IHooks.sol | 5 +- src/libraries/Hooks.sol | 50 ++++++++++++------- src/test/BaseTestHooks.sol | 3 +- src/test/CustomCurveHook.sol | 20 +++----- src/test/DeltaReturningHook.sol | 28 +++++++---- src/test/DynamicFeesTestHook.sol | 5 +- src/test/EmptyTestHooks.sol | 5 +- src/test/MockHooks.sol | 6 ++- src/test/PoolSwapTest.sol | 4 +- src/test/SkipCallsTestHook.sol | 5 +- src/types/BeforeSwapDelta.sol | 40 +++++++++++++++ test/PoolManager.t.sol | 14 +----- 29 files changed, 139 insertions(+), 83 deletions(-) create mode 100644 src/types/BeforeSwapDelta.sol diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap index 97c57bf50..dff59e91d 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -23529 \ No newline at end of file +23609 \ No newline at end of file diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap index 6dae873bb..70853a19b 100644 --- a/.forge-snapshots/simple swap with native.snap +++ b/.forge-snapshots/simple swap with native.snap @@ -1 +1 @@ -117440 \ No newline at end of file +117512 \ No newline at end of file diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap index 7652dce80..ac769f7c2 100644 --- a/.forge-snapshots/simple swap.snap +++ b/.forge-snapshots/simple swap.snap @@ -1 +1 @@ -132636 \ No newline at end of file +132708 \ No newline at end of file diff --git a/.forge-snapshots/swap CA custom curve + swap noop.snap b/.forge-snapshots/swap CA custom curve + swap noop.snap index 186a55e8b..2dcd00601 100644 --- a/.forge-snapshots/swap CA custom curve + swap noop.snap +++ b/.forge-snapshots/swap CA custom curve + swap noop.snap @@ -1 +1 @@ -139399 \ No newline at end of file +135905 \ No newline at end of file diff --git a/.forge-snapshots/swap CA fee on unspecified.snap b/.forge-snapshots/swap CA fee on unspecified.snap index 221e1f23e..50755dacb 100644 --- a/.forge-snapshots/swap CA fee on unspecified.snap +++ b/.forge-snapshots/swap CA fee on unspecified.snap @@ -1 +1 @@ -184879 \ No newline at end of file +185063 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap index c5a1d73a5..620d5391e 100644 --- a/.forge-snapshots/swap against liquidity with native token.snap +++ b/.forge-snapshots/swap against liquidity with native token.snap @@ -1 +1 @@ -113935 \ No newline at end of file +114007 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap index 7702e4fff..d952c35ca 100644 --- a/.forge-snapshots/swap against liquidity.snap +++ b/.forge-snapshots/swap against liquidity.snap @@ -1 +1 @@ -125313 \ No newline at end of file +125385 \ No newline at end of file diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap index 19f777efe..dbec86e55 100644 --- a/.forge-snapshots/swap burn 6909 for input.snap +++ b/.forge-snapshots/swap burn 6909 for input.snap @@ -1 +1 @@ -137318 \ No newline at end of file +137390 \ No newline at end of file diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap index d4bfbdca3..37da6ccf7 100644 --- a/.forge-snapshots/swap burn native 6909 for input.snap +++ b/.forge-snapshots/swap burn native 6909 for input.snap @@ -1 +1 @@ -126418 \ No newline at end of file +126490 \ No newline at end of file diff --git a/.forge-snapshots/swap mint native output as 6909.snap b/.forge-snapshots/swap mint native output as 6909.snap index f97aa25f1..2ec4b2d30 100644 --- a/.forge-snapshots/swap mint native output as 6909.snap +++ b/.forge-snapshots/swap mint native output as 6909.snap @@ -1 +1 @@ -148453 \ No newline at end of file +148525 \ No newline at end of file diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap index d706786b3..4c7225a90 100644 --- a/.forge-snapshots/swap mint output as 6909.snap +++ b/.forge-snapshots/swap mint output as 6909.snap @@ -1 +1 @@ -165348 \ No newline at end of file +165420 \ No newline at end of file diff --git a/.forge-snapshots/swap skips hook call if hook is caller.snap b/.forge-snapshots/swap skips hook call if hook is caller.snap index b7c3d28f5..b3bd260e3 100644 --- a/.forge-snapshots/swap skips hook call if hook is caller.snap +++ b/.forge-snapshots/swap skips hook call if hook is caller.snap @@ -1 +1 @@ -225042 \ No newline at end of file +225039 \ No newline at end of file diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap index c115de177..a4c3b4ede 100644 --- a/.forge-snapshots/swap with dynamic fee.snap +++ b/.forge-snapshots/swap with dynamic fee.snap @@ -1 +1 @@ -149615 \ No newline at end of file +149687 \ No newline at end of file diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap index 46fbfabfa..7f27730a4 100644 --- a/.forge-snapshots/swap with hooks.snap +++ b/.forge-snapshots/swap with hooks.snap @@ -1 +1 @@ -125325 \ No newline at end of file +125397 \ No newline at end of file diff --git a/.forge-snapshots/swap with lp fee and protocol fee.snap b/.forge-snapshots/swap with lp fee and protocol fee.snap index 0561b4e61..f5c641bec 100644 --- a/.forge-snapshots/swap with lp fee and protocol fee.snap +++ b/.forge-snapshots/swap with lp fee and protocol fee.snap @@ -1 +1 @@ -181967 \ No newline at end of file +181949 \ No newline at end of file diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap index 1a498be51..8776ddfc8 100644 --- a/.forge-snapshots/update dynamic fee in before swap.snap +++ b/.forge-snapshots/update dynamic fee in before swap.snap @@ -1 +1 @@ -160258 \ No newline at end of file +160240 \ No newline at end of file diff --git a/src/PoolManager.sol b/src/PoolManager.sol index 413006c23..d70dce273 100644 --- a/src/PoolManager.sol +++ b/src/PoolManager.sol @@ -17,6 +17,7 @@ import {ProtocolFees} from "./ProtocolFees.sol"; import {ERC6909Claims} from "./ERC6909Claims.sol"; import {PoolId, PoolIdLibrary} from "./types/PoolId.sol"; import {BalanceDelta, BalanceDeltaLibrary, toBalanceDelta} from "./types/BalanceDelta.sol"; +import {BeforeSwapDelta} from "./types/BeforeSwapDelta.sol"; import {Lock} from "./libraries/Lock.sol"; import {CurrencyDelta} from "./libraries/CurrencyDelta.sol"; import {NonZeroDeltaCount} from "./libraries/NonZeroDeltaCount.sol"; @@ -270,7 +271,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim PoolId id = key.toId(); _checkPoolInitialized(id); - (int256 amountToSwap, int128 hookDeltaSpecified) = key.hooks.beforeSwap(key, params, hookData); + (int256 amountToSwap, BeforeSwapDelta beforeSwapDelta) = key.hooks.beforeSwap(key, params, hookData); // execute swap, account protocol fees, and emit swap event swapDelta = _swap( @@ -285,7 +286,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim ); BalanceDelta hookDelta; - (swapDelta, hookDelta) = key.hooks.afterSwap(key, params, swapDelta, hookData, hookDeltaSpecified); + (swapDelta, hookDelta) = key.hooks.afterSwap(key, params, swapDelta, hookData, beforeSwapDelta); // if the hook doesnt have the flag to be able to return deltas, hookDelta will always be 0 if (hookDelta != BalanceDeltaLibrary.ZERO_DELTA) _accountPoolBalanceDelta(key, hookDelta, address(key.hooks)); diff --git a/src/interfaces/IHooks.sol b/src/interfaces/IHooks.sol index 2e64acb3e..a3b3de193 100644 --- a/src/interfaces/IHooks.sol +++ b/src/interfaces/IHooks.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.24; import {PoolKey} from "../types/PoolKey.sol"; import {BalanceDelta} from "../types/BalanceDelta.sol"; import {IPoolManager} from "./IPoolManager.sol"; +import {BeforeSwapDelta} from "../types/BeforeSwapDelta.sol"; /// @notice The PoolManager contract decides whether to invoke specific hooks by inspecting the leading bits /// of the hooks contract address. For example, a 1 bit in the first bit of the address will @@ -97,13 +98,13 @@ interface IHooks { /// @param params The parameters for the swap /// @param hookData Arbitrary data handed into the PoolManager by the swapper to be be passed on to the hook /// @return bytes4 The function selector for the hook - /// @return int128 The hook's delta in specified currency. Positive: the hook is owed/took currency, negative: the hook owes/sent currency + /// @return BeforeSwapDelta The hook's delta in specified and unspecified currencies. Positive: the hook is owed/took currency, negative: the hook owes/sent currency function beforeSwap( address sender, PoolKey calldata key, IPoolManager.SwapParams calldata params, bytes calldata hookData - ) external returns (bytes4, int128); + ) external returns (bytes4, BeforeSwapDelta); /// @notice The hook called after a swap /// @param sender The initial msg.sender for the swap call diff --git a/src/libraries/Hooks.sol b/src/libraries/Hooks.sol index a6cefc69a..5c6d9dff2 100644 --- a/src/libraries/Hooks.sol +++ b/src/libraries/Hooks.sol @@ -6,6 +6,7 @@ import {IHooks} from "../interfaces/IHooks.sol"; import {SafeCast} from "../libraries/SafeCast.sol"; import {LPFeeLibrary} from "./LPFeeLibrary.sol"; import {BalanceDelta, toBalanceDelta, BalanceDeltaLibrary} from "../types/BalanceDelta.sol"; +import {BeforeSwapDelta, BeforeSwapDeltaLibrary} from "../types/BeforeSwapDelta.sol"; import {IPoolManager} from "../interfaces/IPoolManager.sol"; /// @notice V4 decides whether to invoke specific hooks by inspecting the leading bits of the address that @@ -16,6 +17,7 @@ library Hooks { using LPFeeLibrary for uint24; using Hooks for IHooks; using SafeCast for int256; + using BeforeSwapDeltaLibrary for BeforeSwapDelta; uint256 internal constant BEFORE_INITIALIZE_FLAG = 1 << 159; uint256 internal constant AFTER_INITIALIZE_FLAG = 1 << 158; @@ -141,6 +143,7 @@ library Hooks { { bytes memory result = callHook(self, data); + // If this hook wasnt meant to return something, default to 0 delta if (!parseReturn) return 0; (, delta) = abi.decode(result, (bytes4, int256)); } @@ -233,21 +236,31 @@ library Hooks { /// @notice calls beforeSwap hook if permissioned and validates return value function beforeSwap(IHooks self, PoolKey memory key, IPoolManager.SwapParams memory params, bytes calldata hookData) internal - returns (int256 amountToSwap, int128 hookDeltaSpecified) + returns (int256 amountToSwap, BeforeSwapDelta hookReturn) { amountToSwap = params.amountSpecified; - if (msg.sender == address(self)) return (amountToSwap, hookDeltaSpecified); + if (msg.sender == address(self)) return (amountToSwap, BeforeSwapDeltaLibrary.ZERO_DELTA); + if (self.hasPermission(BEFORE_SWAP_FLAG)) { - hookDeltaSpecified = self.callHookWithReturnDelta( - abi.encodeWithSelector(IHooks.beforeSwap.selector, msg.sender, key, params, hookData), - self.hasPermission(BEFORE_SWAP_RETURNS_DELTA_FLAG) - ).toInt128(); + bool canReturnDelta = self.hasPermission(BEFORE_SWAP_RETURNS_DELTA_FLAG); + hookReturn = BeforeSwapDelta.wrap( + self.callHookWithReturnDelta( + abi.encodeWithSelector(IHooks.beforeSwap.selector, msg.sender, key, params, hookData), + canReturnDelta + ) + ); - // Update the swap amount according to the hook's return, and check that the swap type doesnt change (exact input/output) - if (hookDeltaSpecified != 0) { - bool exactInput = amountToSwap < 0; - amountToSwap += hookDeltaSpecified; - if (exactInput ? amountToSwap > 0 : amountToSwap < 0) revert HookDeltaExceedsSwapAmount(); + // skip this logic for the case where the hook return is 0 + if (canReturnDelta) { + // any return in unspecified is passed to the afterSwap hook for handling + int128 hookDeltaSpecified = hookReturn.getSpecifiedDelta(); + + // Update the swap amount according to the hook's return, and check that the swap type doesnt change (exact input/output) + if (hookDeltaSpecified != 0) { + bool exactInput = amountToSwap < 0; + amountToSwap += hookDeltaSpecified; + if (exactInput ? amountToSwap > 0 : amountToSwap < 0) revert HookDeltaExceedsSwapAmount(); + } } } } @@ -259,27 +272,30 @@ library Hooks { IPoolManager.SwapParams memory params, BalanceDelta swapDelta, bytes calldata hookData, - int128 hookDeltaSpecified - ) internal returns (BalanceDelta swapperDelta, BalanceDelta hookDelta) { + BeforeSwapDelta beforeSwapHookReturn + ) internal returns (BalanceDelta, BalanceDelta) { if (msg.sender == address(self)) return (swapDelta, BalanceDeltaLibrary.ZERO_DELTA); - int128 hookDeltaUnspecified; - swapperDelta = swapDelta; + int128 hookDeltaSpecified = beforeSwapHookReturn.getSpecifiedDelta(); + int128 hookDeltaUnspecified = beforeSwapHookReturn.getUnspecifiedDelta(); + if (self.hasPermission(AFTER_SWAP_FLAG)) { - hookDeltaUnspecified = self.callHookWithReturnDelta( + hookDeltaUnspecified += self.callHookWithReturnDelta( abi.encodeWithSelector(IHooks.afterSwap.selector, msg.sender, key, params, swapDelta, hookData), self.hasPermission(AFTER_SWAP_RETURNS_DELTA_FLAG) ).toInt128(); } + BalanceDelta hookDelta; if (hookDeltaUnspecified != 0 || hookDeltaSpecified != 0) { hookDelta = (params.amountSpecified < 0 == params.zeroForOne) ? toBalanceDelta(hookDeltaSpecified, hookDeltaUnspecified) : toBalanceDelta(hookDeltaUnspecified, hookDeltaSpecified); // the caller has to pay for (or receive) the hook's delta - swapperDelta = swapDelta - hookDelta; + swapDelta = swapDelta - hookDelta; } + return (swapDelta, hookDelta); } /// @notice calls beforeDonate hook if permissioned and validates return value diff --git a/src/test/BaseTestHooks.sol b/src/test/BaseTestHooks.sol index be791a6e7..061887dc2 100644 --- a/src/test/BaseTestHooks.sol +++ b/src/test/BaseTestHooks.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.24; import {IHooks} from "../interfaces/IHooks.sol"; import {PoolKey} from "../types/PoolKey.sol"; import {BalanceDelta} from "../types/BalanceDelta.sol"; +import {BeforeSwapDelta} from "../types/BeforeSwapDelta.sol"; import {IPoolManager} from "../interfaces/IPoolManager.sol"; contract BaseTestHooks is IHooks { @@ -71,7 +72,7 @@ contract BaseTestHooks is IHooks { PoolKey calldata, /* key **/ IPoolManager.SwapParams calldata, /* params **/ bytes calldata /* hookData **/ - ) external virtual returns (bytes4, int128) { + ) external virtual returns (bytes4, BeforeSwapDelta) { revert HookNotImplemented(); } diff --git a/src/test/CustomCurveHook.sol b/src/test/CustomCurveHook.sol index ab7f2725e..1135e380d 100644 --- a/src/test/CustomCurveHook.sol +++ b/src/test/CustomCurveHook.sol @@ -6,7 +6,8 @@ import {SafeCast} from "../libraries/SafeCast.sol"; import {IHooks} from "../interfaces/IHooks.sol"; import {IPoolManager} from "../interfaces/IPoolManager.sol"; import {PoolKey} from "../types/PoolKey.sol"; -import {BalanceDelta, toBalanceDelta} from "../types/BalanceDelta.sol"; +import {BeforeSwapDelta, toBeforeSwapDelta} from "../types/BeforeSwapDelta.sol"; +import {BalanceDelta} from "../types/BalanceDelta.sol"; import {Currency} from "../types/Currency.sol"; import {CurrencySettleTake} from "../libraries/CurrencySettleTake.sol"; import {BaseTestHooks} from "./BaseTestHooks.sol"; @@ -36,7 +37,7 @@ contract CustomCurveHook is BaseTestHooks { PoolKey calldata key, IPoolManager.SwapParams calldata params, bytes calldata /* hookData **/ - ) external override onlyPoolManager returns (bytes4, int128) { + ) external override onlyPoolManager returns (bytes4, BeforeSwapDelta) { (Currency inputCurrency, Currency outputCurrency, uint256 amount) = _getInputOutputAndAmount(key, params); // this "custom curve" is a line, 1-1 @@ -44,18 +45,9 @@ contract CustomCurveHook is BaseTestHooks { manager.take(inputCurrency, address(this), amount); outputCurrency.settle(manager, address(this), amount, false); - // return -amountSpecified to no-op the concentrated liquidity swap - return (IHooks.beforeSwap.selector, int128(-params.amountSpecified)); - } - - function afterSwap( - address, /* sender **/ - PoolKey calldata, /* key **/ - IPoolManager.SwapParams calldata params, - BalanceDelta, /* delta **/ - bytes calldata /* hookData **/ - ) external view override onlyPoolManager returns (bytes4, int128) { - return (IHooks.afterSwap.selector, int128(params.amountSpecified)); + // return -amountSpecified as specified to no-op the concentrated liquidity swap + BeforeSwapDelta hookDelta = toBeforeSwapDelta(int128(-params.amountSpecified), int128(params.amountSpecified)); + return (IHooks.beforeSwap.selector, hookDelta); } function afterAddLiquidity( diff --git a/src/test/DeltaReturningHook.sol b/src/test/DeltaReturningHook.sol index 9a093caf6..d8b7e2d9e 100644 --- a/src/test/DeltaReturningHook.sol +++ b/src/test/DeltaReturningHook.sol @@ -12,6 +12,7 @@ import {Currency} from "../types/Currency.sol"; import {BaseTestHooks} from "./BaseTestHooks.sol"; import {IERC20Minimal} from "../interfaces/external/IERC20Minimal.sol"; import {CurrencyLibrary, Currency} from "../types/Currency.sol"; +import {BeforeSwapDelta, toBeforeSwapDelta} from "../types/BeforeSwapDelta.sol"; contract DeltaReturningHook is BaseTestHooks { using Hooks for IHooks; @@ -21,7 +22,8 @@ contract DeltaReturningHook is BaseTestHooks { IPoolManager immutable manager; int128 deltaSpecified; - int128 deltaUnspecified; + int128 deltaUnspecifiedBeforeSwap; + int128 deltaUnspecifiedAfterSwap; constructor(IPoolManager _manager) { manager = _manager; @@ -36,8 +38,12 @@ contract DeltaReturningHook is BaseTestHooks { deltaSpecified = delta; } - function setDeltaUnspecified(int128 delta) external { - deltaUnspecified = delta; + function setDeltaUnspecifiedBeforeSwap(int128 delta) external { + deltaUnspecifiedBeforeSwap = delta; + } + + function setDeltaUnspecifiedAfterSwap(int128 delta) external { + deltaUnspecifiedAfterSwap = delta; } function beforeSwap( @@ -45,11 +51,15 @@ contract DeltaReturningHook is BaseTestHooks { PoolKey calldata key, IPoolManager.SwapParams calldata params, bytes calldata /* hookData **/ - ) external override onlyPoolManager returns (bytes4, int128) { - (Currency specifiedCurrency,) = _sortCurrencies(key, params); - _settleOrTake(specifiedCurrency, deltaSpecified); + ) external override onlyPoolManager returns (bytes4, BeforeSwapDelta) { + (Currency specifiedCurrency, Currency unspecifiedCurrency) = _sortCurrencies(key, params); + + if (deltaSpecified != 0) _settleOrTake(specifiedCurrency, deltaSpecified); + if (deltaUnspecifiedBeforeSwap != 0) _settleOrTake(unspecifiedCurrency, deltaUnspecifiedBeforeSwap); + + BeforeSwapDelta beforeSwapDelta = toBeforeSwapDelta(deltaSpecified, deltaUnspecifiedBeforeSwap); - return (IHooks.beforeSwap.selector, deltaSpecified); + return (IHooks.beforeSwap.selector, beforeSwapDelta); } function afterSwap( @@ -60,9 +70,9 @@ contract DeltaReturningHook is BaseTestHooks { bytes calldata /* hookData **/ ) external override onlyPoolManager returns (bytes4, int128) { (, Currency unspecifiedCurrency) = _sortCurrencies(key, params); - _settleOrTake(unspecifiedCurrency, deltaUnspecified); + _settleOrTake(unspecifiedCurrency, deltaUnspecifiedAfterSwap); - return (IHooks.afterSwap.selector, deltaUnspecified); + return (IHooks.afterSwap.selector, deltaUnspecifiedAfterSwap); } function _sortCurrencies(PoolKey calldata key, IPoolManager.SwapParams calldata params) diff --git a/src/test/DynamicFeesTestHook.sol b/src/test/DynamicFeesTestHook.sol index c6355d4b9..f0fc00f0e 100644 --- a/src/test/DynamicFeesTestHook.sol +++ b/src/test/DynamicFeesTestHook.sol @@ -5,6 +5,7 @@ import {BaseTestHooks} from "./BaseTestHooks.sol"; import {PoolKey} from "../types/PoolKey.sol"; import {IPoolManager} from "../interfaces/IPoolManager.sol"; import {IHooks} from "../interfaces/IHooks.sol"; +import {BeforeSwapDelta, BeforeSwapDeltaLibrary} from "../types/BeforeSwapDelta.sol"; contract DynamicFeesTestHook is BaseTestHooks { uint24 internal fee; @@ -30,10 +31,10 @@ contract DynamicFeesTestHook is BaseTestHooks { function beforeSwap(address, PoolKey calldata key, IPoolManager.SwapParams calldata, bytes calldata) external override - returns (bytes4, int128) + returns (bytes4, BeforeSwapDelta) { manager.updateDynamicLPFee(key, fee); - return (IHooks.beforeSwap.selector, 0); + return (IHooks.beforeSwap.selector, BeforeSwapDeltaLibrary.ZERO_DELTA); } function forcePoolFeeUpdate(PoolKey calldata _key, uint24 _fee) external { diff --git a/src/test/EmptyTestHooks.sol b/src/test/EmptyTestHooks.sol index 0b3ed4128..9f49042e2 100644 --- a/src/test/EmptyTestHooks.sol +++ b/src/test/EmptyTestHooks.sol @@ -6,6 +6,7 @@ import {IHooks} from "../interfaces/IHooks.sol"; import {IPoolManager} from "../interfaces/IPoolManager.sol"; import {PoolKey} from "../types/PoolKey.sol"; import {BalanceDelta, BalanceDeltaLibrary} from "../types/BalanceDelta.sol"; +import {BeforeSwapDelta, BeforeSwapDeltaLibrary} from "../types/BeforeSwapDelta.sol"; contract EmptyTestHooks is IHooks { using Hooks for IHooks; @@ -91,9 +92,9 @@ contract EmptyTestHooks is IHooks { external pure override - returns (bytes4, int128) + returns (bytes4, BeforeSwapDelta) { - return (IHooks.beforeSwap.selector, 0); + return (IHooks.beforeSwap.selector, BeforeSwapDeltaLibrary.ZERO_DELTA); } function afterSwap(address, PoolKey calldata, IPoolManager.SwapParams calldata, BalanceDelta, bytes calldata) diff --git a/src/test/MockHooks.sol b/src/test/MockHooks.sol index e7dc57f09..07f5af50d 100644 --- a/src/test/MockHooks.sol +++ b/src/test/MockHooks.sol @@ -7,6 +7,7 @@ import {IPoolManager} from "../interfaces/IPoolManager.sol"; import {PoolKey} from "../types/PoolKey.sol"; import {BalanceDelta, BalanceDeltaLibrary} from "../types/BalanceDelta.sol"; import {PoolId, PoolIdLibrary} from "../types/PoolId.sol"; +import {BeforeSwapDelta, BeforeSwapDeltaLibrary} from "../types/BeforeSwapDelta.sol"; contract MockHooks is IHooks { using PoolIdLibrary for PoolKey; @@ -96,11 +97,12 @@ contract MockHooks is IHooks { function beforeSwap(address, PoolKey calldata, IPoolManager.SwapParams calldata, bytes calldata hookData) external override - returns (bytes4, int128) + returns (bytes4, BeforeSwapDelta) { beforeSwapData = hookData; bytes4 selector = MockHooks.beforeSwap.selector; - return (returnValues[selector] == bytes4(0) ? selector : returnValues[selector], 0); + return + (returnValues[selector] == bytes4(0) ? selector : returnValues[selector], BeforeSwapDeltaLibrary.ZERO_DELTA); } function afterSwap( diff --git a/src/test/PoolSwapTest.sol b/src/test/PoolSwapTest.sol index b1bdd5b24..0664a3d02 100644 --- a/src/test/PoolSwapTest.sol +++ b/src/test/PoolSwapTest.sol @@ -73,7 +73,7 @@ contract PoolSwapTest is PoolTestBase { require(deltaAfter1 >= 0, "deltaAfter1 is not greater than or equal to 0"); } else { // exact output, 0 for 1 - require(deltaAfter0 <= 0, "deltaAfter0 is not less than zero"); + require(deltaAfter0 <= 0, "deltaAfter0 is not less than or equal to zero"); require(delta.amount1() == deltaAfter1, "delta.amount1() is not equal to deltaAfter1"); require( deltaAfter1 <= data.params.amountSpecified, @@ -91,7 +91,7 @@ contract PoolSwapTest is PoolTestBase { require(deltaAfter0 >= 0, "deltaAfter0 is not greater than or equal to 0"); } else { // exact output, 1 for 0 - require(deltaAfter1 <= 0, "deltaAfter1 is not less than 0"); + require(deltaAfter1 <= 0, "deltaAfter1 is not less than or equal to 0"); require(delta.amount0() == deltaAfter0, "delta.amount0() is not equal to deltaAfter0"); require( deltaAfter0 <= data.params.amountSpecified, diff --git a/src/test/SkipCallsTestHook.sol b/src/test/SkipCallsTestHook.sol index 7d0a8f2c9..50035ca96 100644 --- a/src/test/SkipCallsTestHook.sol +++ b/src/test/SkipCallsTestHook.sol @@ -14,6 +14,7 @@ import {PoolTestBase} from "./PoolTestBase.sol"; import {Constants} from "../../test/utils/Constants.sol"; import {Test} from "forge-std/Test.sol"; import {CurrencySettleTake} from "../libraries/CurrencySettleTake.sol"; +import {BeforeSwapDelta, BeforeSwapDeltaLibrary} from "../types/BeforeSwapDelta.sol"; contract SkipCallsTestHook is BaseTestHooks, Test { using CurrencySettleTake for Currency; @@ -96,11 +97,11 @@ contract SkipCallsTestHook is BaseTestHooks, Test { function beforeSwap(address, PoolKey calldata key, IPoolManager.SwapParams calldata params, bytes calldata hookData) external override - returns (bytes4, int128) + returns (bytes4, BeforeSwapDelta) { counter++; _swap(key, params, hookData); - return (IHooks.beforeSwap.selector, 0); + return (IHooks.beforeSwap.selector, BeforeSwapDeltaLibrary.ZERO_DELTA); } function afterSwap( diff --git a/src/types/BeforeSwapDelta.sol b/src/types/BeforeSwapDelta.sol new file mode 100644 index 000000000..d7b7bdd6c --- /dev/null +++ b/src/types/BeforeSwapDelta.sol @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {BalanceDelta} from "./BalanceDelta.sol"; + +// Return type of the beforeSwap hook. +// Upper 128 bits is the delta in specified tokens. Lower 128 bits is delta in unspecified tokens (to match the afterSwap hook) +type BeforeSwapDelta is int256; + +// Creates a BeforeSwapDelta from specified and unspecified +function toBeforeSwapDelta(int128 deltaSpecified, int128 deltaUnspecified) + pure + returns (BeforeSwapDelta beforeSwapDelta) +{ + /// @solidity memory-safe-assembly + assembly { + beforeSwapDelta := or(shl(128, deltaSpecified), and(sub(shl(128, 1), 1), deltaUnspecified)) + } +} + +library BeforeSwapDeltaLibrary { + BeforeSwapDelta public constant ZERO_DELTA = BeforeSwapDelta.wrap(0); + + /// extracts int128 from the upper 128 bits of the BeforeSwapDelta + /// returned by beforeSwap + function getSpecifiedDelta(BeforeSwapDelta delta) internal pure returns (int128 deltaSpecified) { + assembly { + deltaSpecified := sar(128, delta) + } + } + + /// extracts int128 from the lower 128 bits of the BeforeSwapDelta + /// returned by beforeSwap and afterSwap + function getUnspecifiedDelta(BeforeSwapDelta delta) internal pure returns (int128 deltaUnspecified) { + /// @solidity memory-safe-assembly + assembly { + deltaUnspecified := signextend(15, delta) + } + } +} diff --git a/test/PoolManager.t.sol b/test/PoolManager.t.sol index 75aff2973..3d063462c 100644 --- a/test/PoolManager.t.sol +++ b/test/PoolManager.t.sol @@ -876,12 +876,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { } function test_swap_beforeSwapNoOpsSwap_exactInput() public { - address hookAddr = address( - uint160( - Hooks.AFTER_SWAP_FLAG | Hooks.AFTER_SWAP_RETURNS_DELTA_FLAG | Hooks.BEFORE_SWAP_FLAG - | Hooks.BEFORE_SWAP_RETURNS_DELTA_FLAG - ) - ); + address hookAddr = address(uint160(Hooks.BEFORE_SWAP_FLAG | Hooks.BEFORE_SWAP_RETURNS_DELTA_FLAG)); CustomCurveHook impl = new CustomCurveHook(manager); vm.etch(hookAddr, address(impl).code); @@ -910,12 +905,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { } function test_swap_beforeSwapNoOpsSwap_exactOutput() public { - address hookAddr = address( - uint160( - Hooks.AFTER_SWAP_FLAG | Hooks.AFTER_SWAP_RETURNS_DELTA_FLAG | Hooks.BEFORE_SWAP_FLAG - | Hooks.BEFORE_SWAP_RETURNS_DELTA_FLAG - ) - ); + address hookAddr = address(uint160(Hooks.BEFORE_SWAP_FLAG | Hooks.BEFORE_SWAP_RETURNS_DELTA_FLAG)); CustomCurveHook impl = new CustomCurveHook(manager); vm.etch(hookAddr, address(impl).code); From 0f5d73947685bb8e154bf0f44060a3a55543e63a Mon Sep 17 00:00:00 2001 From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com> Date: Mon, 13 May 2024 17:50:53 -0500 Subject: [PATCH 6/9] Move balance delta assignment into unchecked block in `Pool.donate` (#643) * Move balance delta assignment into unchecked block in `Pool.donate` The balance delta calculation has been moved inside an unchecked block in the 'donate' function since overflow isn't possible. * Add comment --- .forge-snapshots/donate gas with 1 token.snap | 2 +- .forge-snapshots/donate gas with 2 tokens.snap | 2 +- .forge-snapshots/poolManager bytecode size.snap | 2 +- src/libraries/Pool.sol | 3 ++- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.forge-snapshots/donate gas with 1 token.snap b/.forge-snapshots/donate gas with 1 token.snap index 835237096..793ef9d3a 100644 --- a/.forge-snapshots/donate gas with 1 token.snap +++ b/.forge-snapshots/donate gas with 1 token.snap @@ -1 +1 @@ -108887 \ No newline at end of file +108741 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 2 tokens.snap b/.forge-snapshots/donate gas with 2 tokens.snap index f89390d96..be21a5835 100644 --- a/.forge-snapshots/donate gas with 2 tokens.snap +++ b/.forge-snapshots/donate gas with 2 tokens.snap @@ -1 +1 @@ -149382 \ No newline at end of file +149236 \ No newline at end of file diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap index dff59e91d..a80672c07 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -23609 \ No newline at end of file +23579 \ No newline at end of file diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol index d3f55d49f..73d3a6277 100644 --- a/src/libraries/Pool.sol +++ b/src/libraries/Pool.sol @@ -473,8 +473,9 @@ library Pool { function donate(State storage state, uint256 amount0, uint256 amount1) internal returns (BalanceDelta delta) { uint128 liquidity = state.liquidity; if (liquidity == 0) revert NoLiquidityToReceiveFees(); - delta = toBalanceDelta(-(amount0.toInt128()), -(amount1.toInt128())); unchecked { + // negation safe as amount0 and amount1 are always positive + delta = toBalanceDelta(-(amount0.toInt128()), -(amount1.toInt128())); if (amount0 > 0) { state.feeGrowthGlobal0X128 += FullMath.mulDiv(amount0, FixedPoint128.Q128, liquidity); } From 5bcb2e65216b9b3bf90df35fee34533311986f77 Mon Sep 17 00:00:00 2001 From: Daniel Gretzke Date: Tue, 14 May 2024 18:30:52 +0200 Subject: [PATCH 7/9] Sparse extsload (#647) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add sparse extsload (#625) * add spare extsload * Add new extsload method to interface * ♻️ Update gas snapshots --------- Co-authored-by: Daniel Gretzke * regenerate snapshots * hex => decimal * regenerate gas snapshots * Use @shuhuiluo's sparse extsload implementation * Add comment explaining the usage of assembly * fix typo * regenerate gas snapshots * Snapshot sparse extsload gas --------- Co-authored-by: Philippe Dumonet --- ...o already existing position with salt.snap | 2 +- .forge-snapshots/addLiquidity CA fee.snap | 2 +- .../addLiquidity with empty hook.snap | 2 +- .../addLiquidity with native token.snap | 2 +- .forge-snapshots/addLiquidity.snap | 2 +- ...new liquidity to a position with salt.snap | 2 +- .forge-snapshots/donate gas with 1 token.snap | 2 +- .../donate gas with 2 tokens.snap | 2 +- .../poolManager bytecode size.snap | 2 +- .forge-snapshots/removeLiquidity CA fee.snap | 2 +- .../removeLiquidity with empty hook.snap | 2 +- .../removeLiquidity with native token.snap | 2 +- .forge-snapshots/removeLiquidity.snap | 2 +- .forge-snapshots/simple swap.snap | 2 +- .forge-snapshots/sparse external sload.snap | 1 + .../swap CA custom curve + swap noop.snap | 2 +- .../swap CA fee on unspecified.snap | 2 +- ...p against liquidity with native token.snap | 2 +- .forge-snapshots/swap against liquidity.snap | 2 +- .../swap burn 6909 for input.snap | 2 +- .../swap burn native 6909 for input.snap | 2 +- .../swap mint native output as 6909.snap | 2 +- .../swap mint output as 6909.snap | 2 +- ...wap skips hook call if hook is caller.snap | 2 +- .forge-snapshots/swap with dynamic fee.snap | 2 +- .forge-snapshots/swap with hooks.snap | 2 +- .../swap with lp fee and protocol fee.snap | 2 +- .../update dynamic fee in before swap.snap | 2 +- src/Extsload.sol | 19 +++++++++++++ src/interfaces/IExtsload.sol | 5 ++++ test/Extsload.t.sol | 28 +++++++++++++++++++ 31 files changed, 80 insertions(+), 27 deletions(-) create mode 100644 .forge-snapshots/sparse external sload.snap create mode 100644 test/Extsload.t.sol diff --git a/.forge-snapshots/add liquidity to already existing position with salt.snap b/.forge-snapshots/add liquidity to already existing position with salt.snap index 57db8502d..f2eceebf0 100644 --- a/.forge-snapshots/add liquidity to already existing position with salt.snap +++ b/.forge-snapshots/add liquidity to already existing position with salt.snap @@ -1 +1 @@ -151132 \ No newline at end of file +151174 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap index 71d1ed3ad..66f302cfc 100644 --- a/.forge-snapshots/addLiquidity CA fee.snap +++ b/.forge-snapshots/addLiquidity CA fee.snap @@ -1 +1 @@ -329498 \ No newline at end of file +329540 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap index 2fbc5708e..a880d226c 100644 --- a/.forge-snapshots/addLiquidity with empty hook.snap +++ b/.forge-snapshots/addLiquidity with empty hook.snap @@ -1 +1 @@ -284133 \ No newline at end of file +284175 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap index b0f1b6580..e2a7016e7 100644 --- a/.forge-snapshots/addLiquidity with native token.snap +++ b/.forge-snapshots/addLiquidity with native token.snap @@ -1 +1 @@ -141308 \ No newline at end of file +141307 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity.snap b/.forge-snapshots/addLiquidity.snap index b12a0aeb1..0957ca694 100644 --- a/.forge-snapshots/addLiquidity.snap +++ b/.forge-snapshots/addLiquidity.snap @@ -1 +1 @@ -151108 \ No newline at end of file +151150 \ No newline at end of file diff --git a/.forge-snapshots/create new liquidity to a position with salt.snap b/.forge-snapshots/create new liquidity to a position with salt.snap index 8811db740..d058bcee5 100644 --- a/.forge-snapshots/create new liquidity to a position with salt.snap +++ b/.forge-snapshots/create new liquidity to a position with salt.snap @@ -1 +1 @@ -299660 \ No newline at end of file +299702 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 1 token.snap b/.forge-snapshots/donate gas with 1 token.snap index 793ef9d3a..5c41fcb36 100644 --- a/.forge-snapshots/donate gas with 1 token.snap +++ b/.forge-snapshots/donate gas with 1 token.snap @@ -1 +1 @@ -108741 \ No newline at end of file +108696 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 2 tokens.snap b/.forge-snapshots/donate gas with 2 tokens.snap index be21a5835..889ec57a3 100644 --- a/.forge-snapshots/donate gas with 2 tokens.snap +++ b/.forge-snapshots/donate gas with 2 tokens.snap @@ -1 +1 @@ -149236 \ No newline at end of file +149234 \ No newline at end of file diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap index a80672c07..57601b4f4 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -23579 \ No newline at end of file +23858 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity CA fee.snap b/.forge-snapshots/removeLiquidity CA fee.snap index 5f9b1f75e..ccf2ce447 100644 --- a/.forge-snapshots/removeLiquidity CA fee.snap +++ b/.forge-snapshots/removeLiquidity CA fee.snap @@ -1 +1 @@ -185071 \ No newline at end of file +185027 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap index b7f06d78c..32f98efe0 100644 --- a/.forge-snapshots/removeLiquidity with empty hook.snap +++ b/.forge-snapshots/removeLiquidity with empty hook.snap @@ -1 +1 @@ -121089 \ No newline at end of file +121045 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap index 3524fddc6..93c0f3d2d 100644 --- a/.forge-snapshots/removeLiquidity with native token.snap +++ b/.forge-snapshots/removeLiquidity with native token.snap @@ -1 +1 @@ -117864 \ No newline at end of file +117820 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity.snap b/.forge-snapshots/removeLiquidity.snap index 2ffa02bf4..837de0999 100644 --- a/.forge-snapshots/removeLiquidity.snap +++ b/.forge-snapshots/removeLiquidity.snap @@ -1 +1 @@ -121077 \ No newline at end of file +121033 \ No newline at end of file diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap index ac769f7c2..010793a84 100644 --- a/.forge-snapshots/simple swap.snap +++ b/.forge-snapshots/simple swap.snap @@ -1 +1 @@ -132708 \ No newline at end of file +132751 \ No newline at end of file diff --git a/.forge-snapshots/sparse external sload.snap b/.forge-snapshots/sparse external sload.snap new file mode 100644 index 000000000..cdfeb3679 --- /dev/null +++ b/.forge-snapshots/sparse external sload.snap @@ -0,0 +1 @@ +2023 \ No newline at end of file diff --git a/.forge-snapshots/swap CA custom curve + swap noop.snap b/.forge-snapshots/swap CA custom curve + swap noop.snap index 2dcd00601..dd5e33e5c 100644 --- a/.forge-snapshots/swap CA custom curve + swap noop.snap +++ b/.forge-snapshots/swap CA custom curve + swap noop.snap @@ -1 +1 @@ -135905 \ No newline at end of file +135903 \ No newline at end of file diff --git a/.forge-snapshots/swap CA fee on unspecified.snap b/.forge-snapshots/swap CA fee on unspecified.snap index 50755dacb..d2350df5e 100644 --- a/.forge-snapshots/swap CA fee on unspecified.snap +++ b/.forge-snapshots/swap CA fee on unspecified.snap @@ -1 +1 @@ -185063 \ No newline at end of file +185018 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap index 620d5391e..6767b79b1 100644 --- a/.forge-snapshots/swap against liquidity with native token.snap +++ b/.forge-snapshots/swap against liquidity with native token.snap @@ -1 +1 @@ -114007 \ No newline at end of file +113919 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap index d952c35ca..651ad1a40 100644 --- a/.forge-snapshots/swap against liquidity.snap +++ b/.forge-snapshots/swap against liquidity.snap @@ -1 +1 @@ -125385 \ No newline at end of file +125340 \ No newline at end of file diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap index dbec86e55..e12fdbcd2 100644 --- a/.forge-snapshots/swap burn 6909 for input.snap +++ b/.forge-snapshots/swap burn 6909 for input.snap @@ -1 +1 @@ -137390 \ No newline at end of file +137302 \ No newline at end of file diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap index 37da6ccf7..38cda8546 100644 --- a/.forge-snapshots/swap burn native 6909 for input.snap +++ b/.forge-snapshots/swap burn native 6909 for input.snap @@ -1 +1 @@ -126490 \ No newline at end of file +126402 \ No newline at end of file diff --git a/.forge-snapshots/swap mint native output as 6909.snap b/.forge-snapshots/swap mint native output as 6909.snap index 2ec4b2d30..2de90b885 100644 --- a/.forge-snapshots/swap mint native output as 6909.snap +++ b/.forge-snapshots/swap mint native output as 6909.snap @@ -1 +1 @@ -148525 \ No newline at end of file +148480 \ No newline at end of file diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap index 4c7225a90..c6f46e583 100644 --- a/.forge-snapshots/swap mint output as 6909.snap +++ b/.forge-snapshots/swap mint output as 6909.snap @@ -1 +1 @@ -165420 \ No newline at end of file +165375 \ No newline at end of file diff --git a/.forge-snapshots/swap skips hook call if hook is caller.snap b/.forge-snapshots/swap skips hook call if hook is caller.snap index b3bd260e3..1a6c091bc 100644 --- a/.forge-snapshots/swap skips hook call if hook is caller.snap +++ b/.forge-snapshots/swap skips hook call if hook is caller.snap @@ -1 +1 @@ -225039 \ No newline at end of file +224993 \ No newline at end of file diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap index a4c3b4ede..eed13f498 100644 --- a/.forge-snapshots/swap with dynamic fee.snap +++ b/.forge-snapshots/swap with dynamic fee.snap @@ -1 +1 @@ -149687 \ No newline at end of file +149642 \ No newline at end of file diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap index 7f27730a4..a863190a9 100644 --- a/.forge-snapshots/swap with hooks.snap +++ b/.forge-snapshots/swap with hooks.snap @@ -1 +1 @@ -125397 \ No newline at end of file +125352 \ No newline at end of file diff --git a/.forge-snapshots/swap with lp fee and protocol fee.snap b/.forge-snapshots/swap with lp fee and protocol fee.snap index f5c641bec..ba9f25241 100644 --- a/.forge-snapshots/swap with lp fee and protocol fee.snap +++ b/.forge-snapshots/swap with lp fee and protocol fee.snap @@ -1 +1 @@ -181949 \ No newline at end of file +181904 \ No newline at end of file diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap index 8776ddfc8..b4a3b3a3e 100644 --- a/.forge-snapshots/update dynamic fee in before swap.snap +++ b/.forge-snapshots/update dynamic fee in before swap.snap @@ -1 +1 @@ -160240 \ No newline at end of file +160195 \ No newline at end of file diff --git a/src/Extsload.sol b/src/Extsload.sol index 776243389..7dd04284f 100644 --- a/src/Extsload.sol +++ b/src/Extsload.sol @@ -25,4 +25,23 @@ abstract contract Extsload is IExtsload { return value; } + + /// @dev since the function is external and enters a new call context and exits right after execution, Solidity's memory management convention can be disregarded and a direct slice of memory can be returned + function extsload(bytes32[] calldata slots) external view returns (bytes32[] memory) { + assembly ("memory-safe") { + // abi offset for dynamic array + mstore(0, 0x20) + mstore(0x20, slots.length) + let end := add(0x40, shl(5, slots.length)) + let memptr := 0x40 + let calldataptr := slots.offset + for {} 1 {} { + mstore(memptr, sload(calldataload(calldataptr))) + memptr := add(memptr, 0x20) + calldataptr := add(calldataptr, 0x20) + if iszero(lt(memptr, end)) { break } + } + return(0, end) + } + } } diff --git a/src/interfaces/IExtsload.sol b/src/interfaces/IExtsload.sol index 3b325008d..c11e75dba 100644 --- a/src/interfaces/IExtsload.sol +++ b/src/interfaces/IExtsload.sol @@ -12,4 +12,9 @@ interface IExtsload { /// @param nSlots Number of slots to load into return value /// @return value The value of the sload-ed slots concatenated as dynamic bytes function extsload(bytes32 slot, uint256 nSlots) external view returns (bytes memory value); + + /// @notice Called by external contracts to access sparse pool state + /// @param slots List of slots to SLOAD from. + /// @return values List of loaded values. + function extsload(bytes32[] calldata slots) external view returns (bytes32[] memory values); } diff --git a/test/Extsload.t.sol b/test/Extsload.t.sol new file mode 100644 index 000000000..36e928a6e --- /dev/null +++ b/test/Extsload.t.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import {Test} from "forge-std/Test.sol"; +import {Extsload} from "../src/Extsload.sol"; +import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol"; + +contract Loadable is Extsload {} + +/// @author philogy +contract ExtsloadTest is Test, GasSnapshot { + Loadable loadable = new Loadable(); + + function test_load10_sparse() public { + bytes32[] memory keys = new bytes32[](10); + for (uint256 i = 0; i < keys.length; i++) { + keys[i] = keccak256(abi.encode(i)); + vm.store(address(loadable), keys[i], bytes32(i)); + } + + bytes32[] memory values = loadable.extsload(keys); + snapLastCall("sparse external sload"); + assertEq(values.length, keys.length); + for (uint256 i = 0; i < values.length; i++) { + assertEq(values[i], bytes32(i)); + } + } +} From 568f7c45aed0dfa6ed04601aae18cd768d00b9d3 Mon Sep 17 00:00:00 2001 From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com> Date: Tue, 14 May 2024 12:07:57 -0500 Subject: [PATCH 8/9] Refactor and optimize `Pool.swap` (#603) * Optimize tick transition in `Pool.swap` Pool.sol has been updated with a block of assembly code for the assignment of `state.tick`. This is equivalent but slightly more efficient version of the previous code. Additionally, the snapshot values have been adjusted as a consequence of this change. * Move `exactInput` and `step` declaration * Update `while` condition in `Pool.swap` The `while` condition within the `swap` function of the `Pool` contract has been changed for clarity and ease of understanding. The condition now explicitly checks for equality compared to the previous implementation. All corresponding snapshot files have been updated to reflect this change. * Update tick boundaries conditions in `Pool.swap` * Switch if-else branches in `Pool.swap` This commit swaps the conditional branches in the `Pool.swap` function in `Pool.sol`. The switch leads to changes in calculated snapshots, indicating reduced gas. * Refactor variable assignment in `Pool` The refactoring of variable assignment in `Pool.sol` aims to increase code readability and maintainability. The change includes replacing separate lines of variable assignments with destructuring assignment, where multiple variables are assigned values in a single statement, making the code more concise and efficient. * Refactor fee growth assignment in `Pool.swap` This update simplifies the fee growth assignments in the `swap` method of the `Pool.sol` file. Previously, the fee growth assignments were performed inside a verbose conditional statement. The refactoring condenses this into a single line of code using a ternary operator, improving the code readability and maintainability. * Optimize Pool.sol for reduced bytecode size This commit includes an optimization in the Pool.sol code where a conditional statement was modified to reduce bytecode size. The snapshot of the bytecode size shows a slight decrease due to these changes, thus ensuring a more efficient contract deployment. --- .../poolManager bytecode size.snap | 2 +- .forge-snapshots/simple swap with native.snap | 2 +- .forge-snapshots/simple swap.snap | 2 +- .../swap CA custom curve + swap noop.snap | 2 +- .../swap CA fee on unspecified.snap | 2 +- ...p against liquidity with native token.snap | 2 +- .forge-snapshots/swap against liquidity.snap | 2 +- .../swap burn 6909 for input.snap | 2 +- .../swap burn native 6909 for input.snap | 2 +- .../swap mint native output as 6909.snap | 2 +- .../swap mint output as 6909.snap | 2 +- ...wap skips hook call if hook is caller.snap | 2 +- .forge-snapshots/swap with dynamic fee.snap | 2 +- .forge-snapshots/swap with hooks.snap | 2 +- .../swap with lp fee and protocol fee.snap | 2 +- .../update dynamic fee in before swap.snap | 2 +- src/libraries/Pool.sol | 63 ++++++++++--------- 17 files changed, 48 insertions(+), 47 deletions(-) diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap index 57601b4f4..927367108 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -23858 \ No newline at end of file +23817 \ No newline at end of file diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap index 70853a19b..5267f17bf 100644 --- a/.forge-snapshots/simple swap with native.snap +++ b/.forge-snapshots/simple swap with native.snap @@ -1 +1 @@ -117512 \ No newline at end of file +117381 \ No newline at end of file diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap index 010793a84..bc35833dc 100644 --- a/.forge-snapshots/simple swap.snap +++ b/.forge-snapshots/simple swap.snap @@ -1 +1 @@ -132751 \ No newline at end of file +132620 \ No newline at end of file diff --git a/.forge-snapshots/swap CA custom curve + swap noop.snap b/.forge-snapshots/swap CA custom curve + swap noop.snap index dd5e33e5c..33ed96a1a 100644 --- a/.forge-snapshots/swap CA custom curve + swap noop.snap +++ b/.forge-snapshots/swap CA custom curve + swap noop.snap @@ -1 +1 @@ -135903 \ No newline at end of file +135892 \ No newline at end of file diff --git a/.forge-snapshots/swap CA fee on unspecified.snap b/.forge-snapshots/swap CA fee on unspecified.snap index d2350df5e..473c75dcb 100644 --- a/.forge-snapshots/swap CA fee on unspecified.snap +++ b/.forge-snapshots/swap CA fee on unspecified.snap @@ -1 +1 @@ -185018 \ No newline at end of file +184887 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap index 6767b79b1..df619ebc1 100644 --- a/.forge-snapshots/swap against liquidity with native token.snap +++ b/.forge-snapshots/swap against liquidity with native token.snap @@ -1 +1 @@ -113919 \ No newline at end of file +113834 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap index 651ad1a40..42ec59cb4 100644 --- a/.forge-snapshots/swap against liquidity.snap +++ b/.forge-snapshots/swap against liquidity.snap @@ -1 +1 @@ -125340 \ No newline at end of file +125255 \ No newline at end of file diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap index e12fdbcd2..0663fd6c5 100644 --- a/.forge-snapshots/swap burn 6909 for input.snap +++ b/.forge-snapshots/swap burn 6909 for input.snap @@ -1 +1 @@ -137302 \ No newline at end of file +137257 \ No newline at end of file diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap index 38cda8546..75d6c8585 100644 --- a/.forge-snapshots/swap burn native 6909 for input.snap +++ b/.forge-snapshots/swap burn native 6909 for input.snap @@ -1 +1 @@ -126402 \ No newline at end of file +126357 \ No newline at end of file diff --git a/.forge-snapshots/swap mint native output as 6909.snap b/.forge-snapshots/swap mint native output as 6909.snap index 2de90b885..2f0f3d4b4 100644 --- a/.forge-snapshots/swap mint native output as 6909.snap +++ b/.forge-snapshots/swap mint native output as 6909.snap @@ -1 +1 @@ -148480 \ No newline at end of file +148435 \ No newline at end of file diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap index c6f46e583..d765b4c6a 100644 --- a/.forge-snapshots/swap mint output as 6909.snap +++ b/.forge-snapshots/swap mint output as 6909.snap @@ -1 +1 @@ -165375 \ No newline at end of file +165244 \ No newline at end of file diff --git a/.forge-snapshots/swap skips hook call if hook is caller.snap b/.forge-snapshots/swap skips hook call if hook is caller.snap index 1a6c091bc..11b96cf97 100644 --- a/.forge-snapshots/swap skips hook call if hook is caller.snap +++ b/.forge-snapshots/swap skips hook call if hook is caller.snap @@ -1 +1 @@ -224993 \ No newline at end of file +224777 \ No newline at end of file diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap index eed13f498..1f6dfff04 100644 --- a/.forge-snapshots/swap with dynamic fee.snap +++ b/.forge-snapshots/swap with dynamic fee.snap @@ -1 +1 @@ -149642 \ No newline at end of file +149511 \ No newline at end of file diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap index a863190a9..8eab8b1e5 100644 --- a/.forge-snapshots/swap with hooks.snap +++ b/.forge-snapshots/swap with hooks.snap @@ -1 +1 @@ -125352 \ No newline at end of file +125267 \ No newline at end of file diff --git a/.forge-snapshots/swap with lp fee and protocol fee.snap b/.forge-snapshots/swap with lp fee and protocol fee.snap index ba9f25241..fa7dfe554 100644 --- a/.forge-snapshots/swap with lp fee and protocol fee.snap +++ b/.forge-snapshots/swap with lp fee and protocol fee.snap @@ -1 +1 @@ -181904 \ No newline at end of file +181833 \ No newline at end of file diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap index b4a3b3a3e..a2b21541f 100644 --- a/.forge-snapshots/update dynamic fee in before swap.snap +++ b/.forge-snapshots/update dynamic fee in before swap.snap @@ -1 +1 @@ -160195 \ No newline at end of file +160064 \ No newline at end of file diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol index 73d3a6277..567fc3272 100644 --- a/src/libraries/Pool.sol +++ b/src/libraries/Pool.sol @@ -217,8 +217,7 @@ library Pool { } if (liquidityDelta != 0) { - int24 tick = self.slot0.tick; - uint160 sqrtPriceX96 = self.slot0.sqrtPriceX96; + (int24 tick, uint160 sqrtPriceX96) = (self.slot0.tick, self.slot0.sqrtPriceX96); if (tick < tickLower) { // current tick is below the passed range; liquidity can only become in range by crossing from left to // right, when we'll need _more_ currency0 (it's becoming more valuable) so user must provide it @@ -305,7 +304,6 @@ library Pool { { Slot0 memory slot0Start = self.slot0; bool zeroForOne = params.zeroForOne; - bool exactInput = params.amountSpecified < 0; SwapCache memory cache = SwapCache({ liquidityStart: self.liquidity, @@ -322,6 +320,8 @@ library Pool { swapFee = cache.protocolFee == 0 ? slot0Start.lpFee : uint24(cache.protocolFee).calculateSwapFee(slot0Start.lpFee); + bool exactInput = params.amountSpecified < 0; + if (!exactInput && (swapFee == LPFeeLibrary.MAX_LP_FEE)) { revert InvalidFeeForExactOut(); } @@ -347,16 +347,17 @@ library Pool { StepComputations memory step; // continue swapping as long as we haven't used the entire input/output and haven't reached the price limit - while (state.amountSpecifiedRemaining != 0 && state.sqrtPriceX96 != params.sqrtPriceLimitX96) { + while (!(state.amountSpecifiedRemaining == 0 || state.sqrtPriceX96 == params.sqrtPriceLimitX96)) { step.sqrtPriceStartX96 = state.sqrtPriceX96; (step.tickNext, step.initialized) = self.tickBitmap.nextInitializedTickWithinOneWord(state.tick, params.tickSpacing, zeroForOne); // ensure that we do not overshoot the min/max tick, as the tick bitmap is not aware of these bounds - if (step.tickNext < TickMath.MIN_TICK) { + if (step.tickNext <= TickMath.MIN_TICK) { step.tickNext = TickMath.MIN_TICK; - } else if (step.tickNext > TickMath.MAX_TICK) { + } + if (step.tickNext >= TickMath.MAX_TICK) { step.tickNext = TickMath.MAX_TICK; } @@ -376,17 +377,17 @@ library Pool { swapFee ); - if (exactInput) { - // safe because we test that amountSpecified > amountIn + feeAmount in SwapMath + if (!exactInput) { unchecked { - state.amountSpecifiedRemaining += (step.amountIn + step.feeAmount).toInt256(); + state.amountSpecifiedRemaining -= step.amountOut.toInt256(); } - state.amountCalculated = state.amountCalculated + step.amountOut.toInt256(); + state.amountCalculated = state.amountCalculated - (step.amountIn + step.feeAmount).toInt256(); } else { + // safe because we test that amountSpecified > amountIn + feeAmount in SwapMath unchecked { - state.amountSpecifiedRemaining -= step.amountOut.toInt256(); + state.amountSpecifiedRemaining += (step.amountIn + step.feeAmount).toInt256(); } - state.amountCalculated = state.amountCalculated - (step.amountIn + step.feeAmount).toInt256(); + state.amountCalculated = state.amountCalculated + step.amountOut.toInt256(); } // if the protocol fee is on, calculate how much is owed, decrement feeAmount, and increment protocolFee @@ -413,15 +414,9 @@ library Pool { if (state.sqrtPriceX96 == step.sqrtPriceNextX96) { // if the tick is initialized, run the tick transition if (step.initialized) { - uint256 feeGrowthGlobal0X128; - uint256 feeGrowthGlobal1X128; - if (!zeroForOne) { - feeGrowthGlobal0X128 = self.feeGrowthGlobal0X128; - feeGrowthGlobal1X128 = state.feeGrowthGlobalX128; - } else { - feeGrowthGlobal0X128 = state.feeGrowthGlobalX128; - feeGrowthGlobal1X128 = self.feeGrowthGlobal1X128; - } + (uint256 feeGrowthGlobal0X128, uint256 feeGrowthGlobal1X128) = zeroForOne + ? (state.feeGrowthGlobalX128, self.feeGrowthGlobal1X128) + : (self.feeGrowthGlobal0X128, state.feeGrowthGlobalX128); int128 liquidityNet = Pool.crossTick(self, step.tickNext, feeGrowthGlobal0X128, feeGrowthGlobal1X128); // if we're moving leftward, we interpret liquidityNet as the opposite sign @@ -433,8 +428,14 @@ library Pool { state.liquidity = LiquidityMath.addDelta(state.liquidity, liquidityNet); } + // Equivalent to `state.tick = zeroForOne ? step.tickNext - 1 : step.tickNext;` unchecked { - state.tick = zeroForOne ? step.tickNext - 1 : step.tickNext; + // cannot cast a bool to an int24 in Solidity + int24 _zeroForOne; + assembly { + _zeroForOne := zeroForOne + } + state.tick = step.tickNext - _zeroForOne; } } else if (state.sqrtPriceX96 != step.sqrtPriceStartX96) { // recompute unless we're on a lower tick boundary (i.e. already transitioned ticks), and haven't moved @@ -442,28 +443,28 @@ library Pool { } } - (self.slot0.sqrtPriceX96, self.slot0.tick) = (state.sqrtPriceX96, state.tick); + (self.slot0.tick, self.slot0.sqrtPriceX96) = (state.tick, state.sqrtPriceX96); // update liquidity if it changed if (cache.liquidityStart != state.liquidity) self.liquidity = state.liquidity; // update fee growth global - if (zeroForOne) { - self.feeGrowthGlobal0X128 = state.feeGrowthGlobalX128; - } else { + if (!zeroForOne) { self.feeGrowthGlobal1X128 = state.feeGrowthGlobalX128; + } else { + self.feeGrowthGlobal0X128 = state.feeGrowthGlobalX128; } unchecked { - if (zeroForOne == exactInput) { + if (zeroForOne != exactInput) { result = toBalanceDelta( - (params.amountSpecified - state.amountSpecifiedRemaining).toInt128(), - state.amountCalculated.toInt128() + state.amountCalculated.toInt128(), + (params.amountSpecified - state.amountSpecifiedRemaining).toInt128() ); } else { result = toBalanceDelta( - state.amountCalculated.toInt128(), - (params.amountSpecified - state.amountSpecifiedRemaining).toInt128() + (params.amountSpecified - state.amountSpecifiedRemaining).toInt128(), + state.amountCalculated.toInt128() ); } } From e4271089c5188009f320f527b404e7679e6e89cd Mon Sep 17 00:00:00 2001 From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com> Date: Tue, 14 May 2024 13:53:23 -0500 Subject: [PATCH 9/9] Optimize `SafeCast` (#576) * Add `toUint128` to `SafeCast` and fuzz test In the `SafeCast` library, a new function `toUint128` has been added to support casting uint256 to uint128. Accompanying the update, a new fuzz test for `toUint128` is also added to verify the behavior when `toUint128` encounters a potential overflow condition. The test ensures its correct functionality and robustness against edge cases. * Optimize `SafeCast` with inline assembly Refactor `SafeCast.sol` library using inline assembly for better memory-safe operations. By using inline assembly, the code optimizes casting operations which result in reduced gas usage. These changes affect numerous `.forge-snapshot` files, demonstrating an overall reduction in gas usage across multiple contracts. * Refactor `SafeCast` for better readability The SafeCast library code has been refactored to increase readability. Specifically, repetition of in-line assembly code has been eliminated in favour of a more abstracted '_revertOverflow()' function. This not only simplifies multiple number casting functions but also helps reducing the bytecode size of the contract subtly. * Added `test_toUint128` function to SafeCast tests The new function tests the casting method `toUint128` in the `SafeCast` library. It checks that the casting correctly handles 0, maximum uint128 values, and overflows. * Refactor SafeCast.sol for enhanced code readability The modification simplifies the SafeCast.sol functions to return types directly rather than using a separate variable. Changes have been made to parameter naming for better understanding. Additionally, an overflow check has been added to the toInt128 function for greater accuracy in casting data types. * Refactor and optimize `toInt256` --- .../SwapMath_oneForZero_exactInPartial.snap | 2 +- .../SwapMath_oneForZero_exactOutPartial.snap | 2 +- ...o already existing position with salt.snap | 2 +- .forge-snapshots/addLiquidity CA fee.snap | 2 +- .../addLiquidity with empty hook.snap | 2 +- .../addLiquidity with native token.snap | 2 +- .forge-snapshots/addLiquidity.snap | 2 +- ...new liquidity to a position with salt.snap | 2 +- .forge-snapshots/donate gas with 1 token.snap | 2 +- .../donate gas with 2 tokens.snap | 2 +- ...iceFromInput_zeroForOneEqualsFalseGas.snap | 2 +- ...ceFromOutput_zeroForOneEqualsFalseGas.snap | 2 +- .../poolManager bytecode size.snap | 2 +- .forge-snapshots/removeLiquidity CA fee.snap | 2 +- .../removeLiquidity with empty hook.snap | 2 +- .../removeLiquidity with native token.snap | 2 +- .forge-snapshots/removeLiquidity.snap | 2 +- .forge-snapshots/simple swap with native.snap | 2 +- .forge-snapshots/simple swap.snap | 2 +- .../swap CA custom curve + swap noop.snap | 2 +- .../swap CA fee on unspecified.snap | 2 +- ...p against liquidity with native token.snap | 2 +- .forge-snapshots/swap against liquidity.snap | 2 +- .../swap burn 6909 for input.snap | 2 +- .../swap burn native 6909 for input.snap | 2 +- .../swap mint native output as 6909.snap | 2 +- .../swap mint output as 6909.snap | 2 +- ...wap skips hook call if hook is caller.snap | 2 +- .forge-snapshots/swap with dynamic fee.snap | 2 +- .forge-snapshots/swap with hooks.snap | 2 +- .../swap with lp fee and protocol fee.snap | 2 +- .../update dynamic fee in before swap.snap | 2 +- src/libraries/SafeCast.sol | 60 ++++++++++++------- test/libraries/SafeCast.t.sol | 16 +++++ 34 files changed, 88 insertions(+), 52 deletions(-) diff --git a/.forge-snapshots/SwapMath_oneForZero_exactInPartial.snap b/.forge-snapshots/SwapMath_oneForZero_exactInPartial.snap index 38ef83141..43e6c3379 100644 --- a/.forge-snapshots/SwapMath_oneForZero_exactInPartial.snap +++ b/.forge-snapshots/SwapMath_oneForZero_exactInPartial.snap @@ -1 +1 @@ -3038 \ No newline at end of file +3016 \ No newline at end of file diff --git a/.forge-snapshots/SwapMath_oneForZero_exactOutPartial.snap b/.forge-snapshots/SwapMath_oneForZero_exactOutPartial.snap index e4da9ac19..72cd61fe0 100644 --- a/.forge-snapshots/SwapMath_oneForZero_exactOutPartial.snap +++ b/.forge-snapshots/SwapMath_oneForZero_exactOutPartial.snap @@ -1 +1 @@ -3278 \ No newline at end of file +3256 \ No newline at end of file diff --git a/.forge-snapshots/add liquidity to already existing position with salt.snap b/.forge-snapshots/add liquidity to already existing position with salt.snap index f2eceebf0..17f6e7771 100644 --- a/.forge-snapshots/add liquidity to already existing position with salt.snap +++ b/.forge-snapshots/add liquidity to already existing position with salt.snap @@ -1 +1 @@ -151174 \ No newline at end of file +151104 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap index 66f302cfc..6a9f64f2f 100644 --- a/.forge-snapshots/addLiquidity CA fee.snap +++ b/.forge-snapshots/addLiquidity CA fee.snap @@ -1 +1 @@ -329540 \ No newline at end of file +329444 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap index a880d226c..8ce2b9562 100644 --- a/.forge-snapshots/addLiquidity with empty hook.snap +++ b/.forge-snapshots/addLiquidity with empty hook.snap @@ -1 +1 @@ -284175 \ No newline at end of file +284085 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap index e2a7016e7..b78768771 100644 --- a/.forge-snapshots/addLiquidity with native token.snap +++ b/.forge-snapshots/addLiquidity with native token.snap @@ -1 +1 @@ -141307 \ No newline at end of file +141237 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity.snap b/.forge-snapshots/addLiquidity.snap index 0957ca694..b40672a13 100644 --- a/.forge-snapshots/addLiquidity.snap +++ b/.forge-snapshots/addLiquidity.snap @@ -1 +1 @@ -151150 \ No newline at end of file +151080 \ No newline at end of file diff --git a/.forge-snapshots/create new liquidity to a position with salt.snap b/.forge-snapshots/create new liquidity to a position with salt.snap index d058bcee5..ab53d56de 100644 --- a/.forge-snapshots/create new liquidity to a position with salt.snap +++ b/.forge-snapshots/create new liquidity to a position with salt.snap @@ -1 +1 @@ -299702 \ No newline at end of file +299632 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 1 token.snap b/.forge-snapshots/donate gas with 1 token.snap index 5c41fcb36..efe7a735d 100644 --- a/.forge-snapshots/donate gas with 1 token.snap +++ b/.forge-snapshots/donate gas with 1 token.snap @@ -1 +1 @@ -108696 \ No newline at end of file +108687 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 2 tokens.snap b/.forge-snapshots/donate gas with 2 tokens.snap index 889ec57a3..6a6f4c603 100644 --- a/.forge-snapshots/donate gas with 2 tokens.snap +++ b/.forge-snapshots/donate gas with 2 tokens.snap @@ -1 +1 @@ -149234 \ No newline at end of file +149222 \ No newline at end of file diff --git a/.forge-snapshots/getNextSqrtPriceFromInput_zeroForOneEqualsFalseGas.snap b/.forge-snapshots/getNextSqrtPriceFromInput_zeroForOneEqualsFalseGas.snap index 7dfce3516..23c5f49dc 100644 --- a/.forge-snapshots/getNextSqrtPriceFromInput_zeroForOneEqualsFalseGas.snap +++ b/.forge-snapshots/getNextSqrtPriceFromInput_zeroForOneEqualsFalseGas.snap @@ -1 +1 @@ -594 \ No newline at end of file +572 \ No newline at end of file diff --git a/.forge-snapshots/getNextSqrtPriceFromOutput_zeroForOneEqualsFalseGas.snap b/.forge-snapshots/getNextSqrtPriceFromOutput_zeroForOneEqualsFalseGas.snap index f8f450742..c5befbc75 100644 --- a/.forge-snapshots/getNextSqrtPriceFromOutput_zeroForOneEqualsFalseGas.snap +++ b/.forge-snapshots/getNextSqrtPriceFromOutput_zeroForOneEqualsFalseGas.snap @@ -1 +1 @@ -878 \ No newline at end of file +856 \ No newline at end of file diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap index 927367108..367a81773 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -23817 \ No newline at end of file +23738 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity CA fee.snap b/.forge-snapshots/removeLiquidity CA fee.snap index ccf2ce447..dcc25a494 100644 --- a/.forge-snapshots/removeLiquidity CA fee.snap +++ b/.forge-snapshots/removeLiquidity CA fee.snap @@ -1 +1 @@ -185027 \ No newline at end of file +184931 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap index 32f98efe0..372159cc7 100644 --- a/.forge-snapshots/removeLiquidity with empty hook.snap +++ b/.forge-snapshots/removeLiquidity with empty hook.snap @@ -1 +1 @@ -121045 \ No newline at end of file +120975 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap index 93c0f3d2d..0b5f4b02c 100644 --- a/.forge-snapshots/removeLiquidity with native token.snap +++ b/.forge-snapshots/removeLiquidity with native token.snap @@ -1 +1 @@ -117820 \ No newline at end of file +117750 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity.snap b/.forge-snapshots/removeLiquidity.snap index 837de0999..53dfe042c 100644 --- a/.forge-snapshots/removeLiquidity.snap +++ b/.forge-snapshots/removeLiquidity.snap @@ -1 +1 @@ -121033 \ No newline at end of file +120963 \ No newline at end of file diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap index 5267f17bf..41735a4dd 100644 --- a/.forge-snapshots/simple swap with native.snap +++ b/.forge-snapshots/simple swap with native.snap @@ -1 +1 @@ -117381 \ No newline at end of file +117339 \ No newline at end of file diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap index bc35833dc..4719fd039 100644 --- a/.forge-snapshots/simple swap.snap +++ b/.forge-snapshots/simple swap.snap @@ -1 +1 @@ -132620 \ No newline at end of file +132578 \ No newline at end of file diff --git a/.forge-snapshots/swap CA custom curve + swap noop.snap b/.forge-snapshots/swap CA custom curve + swap noop.snap index 33ed96a1a..612b68dd0 100644 --- a/.forge-snapshots/swap CA custom curve + swap noop.snap +++ b/.forge-snapshots/swap CA custom curve + swap noop.snap @@ -1 +1 @@ -135892 \ No newline at end of file +135860 \ No newline at end of file diff --git a/.forge-snapshots/swap CA fee on unspecified.snap b/.forge-snapshots/swap CA fee on unspecified.snap index 473c75dcb..08f6f6124 100644 --- a/.forge-snapshots/swap CA fee on unspecified.snap +++ b/.forge-snapshots/swap CA fee on unspecified.snap @@ -1 +1 @@ -184887 \ No newline at end of file +184809 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap index df619ebc1..8b56c7d74 100644 --- a/.forge-snapshots/swap against liquidity with native token.snap +++ b/.forge-snapshots/swap against liquidity with native token.snap @@ -1 +1 @@ -113834 \ No newline at end of file +113800 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap index 42ec59cb4..0372bd1c3 100644 --- a/.forge-snapshots/swap against liquidity.snap +++ b/.forge-snapshots/swap against liquidity.snap @@ -1 +1 @@ -125255 \ No newline at end of file +125221 \ No newline at end of file diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap index 0663fd6c5..9ff1f5c77 100644 --- a/.forge-snapshots/swap burn 6909 for input.snap +++ b/.forge-snapshots/swap burn 6909 for input.snap @@ -1 +1 @@ -137257 \ No newline at end of file +137207 \ No newline at end of file diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap index 75d6c8585..4228ebd6b 100644 --- a/.forge-snapshots/swap burn native 6909 for input.snap +++ b/.forge-snapshots/swap burn native 6909 for input.snap @@ -1 +1 @@ -126357 \ No newline at end of file +126323 \ No newline at end of file diff --git a/.forge-snapshots/swap mint native output as 6909.snap b/.forge-snapshots/swap mint native output as 6909.snap index 2f0f3d4b4..3de9f0d73 100644 --- a/.forge-snapshots/swap mint native output as 6909.snap +++ b/.forge-snapshots/swap mint native output as 6909.snap @@ -1 +1 @@ -148435 \ No newline at end of file +148385 \ No newline at end of file diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap index d765b4c6a..f6f049473 100644 --- a/.forge-snapshots/swap mint output as 6909.snap +++ b/.forge-snapshots/swap mint output as 6909.snap @@ -1 +1 @@ -165244 \ No newline at end of file +165202 \ No newline at end of file diff --git a/.forge-snapshots/swap skips hook call if hook is caller.snap b/.forge-snapshots/swap skips hook call if hook is caller.snap index 11b96cf97..95777a5a8 100644 --- a/.forge-snapshots/swap skips hook call if hook is caller.snap +++ b/.forge-snapshots/swap skips hook call if hook is caller.snap @@ -1 +1 @@ -224777 \ No newline at end of file +224701 \ No newline at end of file diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap index 1f6dfff04..72f4ce3f4 100644 --- a/.forge-snapshots/swap with dynamic fee.snap +++ b/.forge-snapshots/swap with dynamic fee.snap @@ -1 +1 @@ -149511 \ No newline at end of file +149469 \ No newline at end of file diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap index 8eab8b1e5..9f4a834a8 100644 --- a/.forge-snapshots/swap with hooks.snap +++ b/.forge-snapshots/swap with hooks.snap @@ -1 +1 @@ -125267 \ No newline at end of file +125233 \ No newline at end of file diff --git a/.forge-snapshots/swap with lp fee and protocol fee.snap b/.forge-snapshots/swap with lp fee and protocol fee.snap index fa7dfe554..7d3d777bd 100644 --- a/.forge-snapshots/swap with lp fee and protocol fee.snap +++ b/.forge-snapshots/swap with lp fee and protocol fee.snap @@ -1 +1 @@ -181833 \ No newline at end of file +181791 \ No newline at end of file diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap index a2b21541f..cf4bf7d64 100644 --- a/.forge-snapshots/update dynamic fee in before swap.snap +++ b/.forge-snapshots/update dynamic fee in before swap.snap @@ -1 +1 @@ -160064 \ No newline at end of file +160022 \ No newline at end of file diff --git a/src/libraries/SafeCast.sol b/src/libraries/SafeCast.sol index 5eb5bd474..8f22bed45 100644 --- a/src/libraries/SafeCast.sol +++ b/src/libraries/SafeCast.sol @@ -6,35 +6,55 @@ pragma solidity ^0.8.20; library SafeCast { error SafeCastOverflow(); + function _revertOverflow() private pure { + /// @solidity memory-safe-assembly + assembly { + // Store the function selector of `SafeCastOverflow()`. + mstore(0x00, 0x93dafdf1) + // Revert with (offset, size). + revert(0x1c, 0x04) + } + } + /// @notice Cast a uint256 to a uint160, revert on overflow - /// @param y The uint256 to be downcasted - /// @return z The downcasted integer, now type uint160 - function toUint160(uint256 y) internal pure returns (uint160 z) { - z = uint160(y); - if (z != y) revert SafeCastOverflow(); + /// @param x The uint256 to be downcasted + /// @return The downcasted integer, now type uint160 + function toUint160(uint256 x) internal pure returns (uint160) { + if (x >= 1 << 160) _revertOverflow(); + return uint160(x); + } + + /// @notice Cast a uint256 to a uint128, revert on overflow + /// @param x The uint256 to be downcasted + /// @return The downcasted integer, now type uint128 + function toUint128(uint256 x) internal pure returns (uint128) { + if (x >= 1 << 128) _revertOverflow(); + return uint128(x); } /// @notice Cast a int256 to a int128, revert on overflow or underflow - /// @param y The int256 to be downcasted - /// @return z The downcasted integer, now type int128 - function toInt128(int256 y) internal pure returns (int128 z) { - z = int128(y); - if (z != y) revert SafeCastOverflow(); + /// @param x The int256 to be downcasted + /// @return The downcasted integer, now type int128 + function toInt128(int256 x) internal pure returns (int128) { + unchecked { + if (((1 << 127) + uint256(x)) >> 128 == uint256(0)) return int128(x); + _revertOverflow(); + } } /// @notice Cast a uint256 to a int256, revert on overflow - /// @param y The uint256 to be casted - /// @return z The casted integer, now type int256 - function toInt256(uint256 y) internal pure returns (int256 z) { - if (y > uint256(type(int256).max)) revert SafeCastOverflow(); - z = int256(y); + /// @param x The uint256 to be casted + /// @return The casted integer, now type int256 + function toInt256(uint256 x) internal pure returns (int256) { + if (int256(x) >= 0) return int256(x); + _revertOverflow(); } /// @notice Cast a uint256 to a int128, revert on overflow - /// @param y The uint256 to be downcasted - /// @return z The downcasted integer, now type int128 - function toInt128(uint256 y) internal pure returns (int128 z) { - if (y > uint128(type(int128).max)) revert SafeCastOverflow(); - z = int128(int256(y)); + /// @param x The uint256 to be downcasted + /// @return The downcasted integer, now type int128 + function toInt128(uint256 x) internal pure returns (int128) { + if (x >= 1 << 127) _revertOverflow(); + return int128(int256(x)); } } diff --git a/test/libraries/SafeCast.t.sol b/test/libraries/SafeCast.t.sol index bc18e2970..40d873f03 100644 --- a/test/libraries/SafeCast.t.sol +++ b/test/libraries/SafeCast.t.sol @@ -22,6 +22,22 @@ contract SafeCastTest is Test { SafeCast.toUint160(type(uint160).max + uint256(1)); } + function test_fuzz_toUint128(uint256 x) public { + if (x <= type(uint128).max) { + assertEq(uint256(SafeCast.toUint128(x)), x); + } else { + vm.expectRevert(SafeCast.SafeCastOverflow.selector); + SafeCast.toUint128(x); + } + } + + function test_toUint128() public { + assertEq(uint256(SafeCast.toUint128(0)), 0); + assertEq(uint256(SafeCast.toUint128(type(uint128).max)), type(uint128).max); + vm.expectRevert(SafeCast.SafeCastOverflow.selector); + SafeCast.toUint128(type(uint128).max + uint256(1)); + } + function test_fuzz_toInt128_fromInt256(int256 x) public { if (x <= type(int128).max && x >= type(int128).min) { assertEq(int256(SafeCast.toInt128(x)), x);