Optimization/avoid unnecessary sload (#73)

* optimization: unify pool initializer checker to save one sload * optimization: extract the gas a bit more by rewriting the revert logic in assembly
pancakeswap · Jun 12, 2024 · f051737 · f051737
1 parent 3a99be9
commit f051737
Show file tree

Hide file tree

Showing 38 changed files with 81 additions and 68 deletions.
diff --git a/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap
@@ -1 +1 @@
-179107
+179055
diff --git a/.forge-snapshots/BinHookTest#testDonateSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testDonateSucceedsWithHook.snap
@@ -1 +1 @@
-182580
+182536
diff --git a/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap
@@ -1 +1 @@
-329193
+329132
diff --git a/.forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap
@@ -1 +1 @@
-190695
+190631
diff --git a/.forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap b/.forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap
@@ -1 +1 @@
-134814
+134765
diff --git a/.forge-snapshots/BinPoolManagerTest#testFuzzUpdateDynamicLPFee.snap b/.forge-snapshots/BinPoolManagerTest#testFuzzUpdateDynamicLPFee.snap
@@ -1 +1 @@
-32311
+32346
diff --git a/.forge-snapshots/BinPoolManagerTest#testFuzz_SetMaxBinStep.snap b/.forge-snapshots/BinPoolManagerTest#testFuzz_SetMaxBinStep.snap
@@ -1 +1 @@
-30548
+30536
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap b/.forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap
@@ -1 +1 @@
-143911
+143850
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap b/.forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap
@@ -1 +1 @@
-290014
+289965
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap b/.forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap
@@ -1 +1 @@
-127941
+127892
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasDonate.snap b/.forge-snapshots/BinPoolManagerTest#testGasDonate.snap
@@ -1 +1 @@
-116371
+116330
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap
@@ -1 +1 @@
-971054
+970993
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap
@@ -1 +1 @@
-329375
+329314
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap
@@ -1 +1 @@
-339361
+339300
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap
@@ -1 +1 @@
-141771
+141710
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasSwapMultipleBins.snap b/.forge-snapshots/BinPoolManagerTest#testGasSwapMultipleBins.snap
@@ -1 +1 @@
-174486
+174422
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasSwapOverBigBinIdGate.snap b/.forge-snapshots/BinPoolManagerTest#testGasSwapOverBigBinIdGate.snap
@@ -1 +1 @@
-180359
+180295
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasSwapSingleBin.snap b/.forge-snapshots/BinPoolManagerTest#testGasSwapSingleBin.snap
@@ -1 +1 @@
-134374
+134310
diff --git a/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap b/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap
@@ -1 +1 @@
-306120
+306059
diff --git a/.forge-snapshots/BinPoolManagerTest#testSetProtocolFee.snap b/.forge-snapshots/BinPoolManagerTest#testSetProtocolFee.snap
@@ -1 +1 @@
-34468
+34492
diff --git a/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap b/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap
@@ -1 +1 @@
-354532
+354468
diff --git a/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap b/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap
@@ -1 +1 @@
-169969
+169905
diff --git a/.forge-snapshots/CLPoolManagerTest#addLiquidity_nativeToken.snap b/.forge-snapshots/CLPoolManagerTest#addLiquidity_nativeToken.snap
@@ -1 +1 @@
-240680
+240616
diff --git a/.forge-snapshots/CLPoolManagerTest#donateBothTokens.snap b/.forge-snapshots/CLPoolManagerTest#donateBothTokens.snap
@@ -1 +1 @@
-165995
+165934
diff --git a/.forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap b/.forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap
@@ -1 +1 @@
-110429
+110368
diff --git a/.forge-snapshots/CLPoolManagerTest#removeLiquidity_toNonEmpty.snap b/.forge-snapshots/CLPoolManagerTest#removeLiquidity_toNonEmpty.snap
@@ -1 +1 @@
-117154
+117090
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_againstLiquidity.snap b/.forge-snapshots/CLPoolManagerTest#swap_againstLiquidity.snap
@@ -1 +1 @@
-133908
+133844
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_leaveSurplusTokenInVault.snap b/.forge-snapshots/CLPoolManagerTest#swap_leaveSurplusTokenInVault.snap
@@ -1 +1 @@
-166673
+166609
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap b/.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap
@@ -1 +1 @@
-154038
+153974
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_simple.snap b/.forge-snapshots/CLPoolManagerTest#swap_simple.snap
@@ -1 +1 @@
-73539
+73475
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_useSurplusTokenAsInput.snap b/.forge-snapshots/CLPoolManagerTest#swap_useSurplusTokenAsInput.snap
@@ -1 +1 @@
-146083
+146019
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_withHooks.snap b/.forge-snapshots/CLPoolManagerTest#swap_withHooks.snap
@@ -1 +1 @@
-89753
+89689
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_withNative.snap b/.forge-snapshots/CLPoolManagerTest#swap_withNative.snap
@@ -1 +1 @@
-73542
+73478
diff --git a/.forge-snapshots/CLPoolManagerTest#testFuzzUpdateDynamicLPFee.snap b/.forge-snapshots/CLPoolManagerTest#testFuzzUpdateDynamicLPFee.snap
@@ -1 +1 @@
-32060
+32107
diff --git a/src/pool-bin/BinPoolManager.sol b/src/pool-bin/BinPoolManager.sol
@@ -134,15 +134,16 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
         if (amountIn == 0) revert InsufficientAmountIn();
 
         PoolId id = key.toId();
-        _checkPoolInitialized(id);
+        BinPool.State storage pool = pools[id];
+        pool.checkPoolInitialized();
 
         (uint128 amountToSwap, BeforeSwapDelta beforeSwapDelta, uint24 lpFeeOverride) =
             BinHooks.beforeSwap(key, swapForY, amountIn, hookData);
 
         /// @dev fix stack too deep
         {
             BinPool.SwapState memory state;
-            (delta, state) = pools[id].swap(
+            (delta, state) = pool.swap(
                 BinPool.SwapParams({
                     swapForY: swapForY,
                     binStep: key.parameters.getBinStep(),
@@ -182,7 +183,8 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
         returns (uint128 amountIn, uint128 amountOutLeft, uint128 fee)
     {
         PoolId id = key.toId();
-        _checkPoolInitialized(id);
+        BinPool.State storage pool = pools[id];
+        pool.checkPoolInitialized();
 
         uint24 lpFee;
         if (key.fee.isDynamicLPFee()) {
@@ -194,7 +196,7 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
         }
         lpFee.validate(LPFeeLibrary.TEN_PERCENT_FEE);
 
-        (amountIn, amountOutLeft, fee) = pools[id].getSwapIn(
+        (amountIn, amountOutLeft, fee) = pool.getSwapIn(
             BinPool.SwapViewParams({swapForY: swapForY, binStep: key.parameters.getBinStep(), lpFee: lpFee}), amountOut
         );
     }
@@ -207,7 +209,8 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
         returns (uint128 amountInLeft, uint128 amountOut, uint128 fee)
     {
         PoolId id = key.toId();
-        _checkPoolInitialized(id);
+        BinPool.State storage pool = pools[id];
+        pool.checkPoolInitialized();
 
         uint24 lpFee;
         if (key.fee.isDynamicLPFee()) {
@@ -218,7 +221,7 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
         }
         lpFee.validate(LPFeeLibrary.TEN_PERCENT_FEE);
 
-        (amountInLeft, amountOut, fee) = pools[id].getSwapOut(
+        (amountInLeft, amountOut, fee) = pool.getSwapOut(
             BinPool.SwapViewParams({swapForY: swapForY, binStep: key.parameters.getBinStep(), lpFee: lpFee}), amountIn
         );
     }
@@ -231,13 +234,14 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
         returns (BalanceDelta delta, BinPool.MintArrays memory mintArray)
     {
         PoolId id = key.toId();
-        _checkPoolInitialized(id);
+        BinPool.State storage pool = pools[id];
+        pool.checkPoolInitialized();
 
         BinHooks.beforeMint(key, params, hookData);
 
         bytes32 feeForProtocol;
         bytes32 compositionFee;
-        (delta, feeForProtocol, mintArray, compositionFee) = pools[id].mint(
+        (delta, feeForProtocol, mintArray, compositionFee) = pool.mint(
             BinPool.MintParams({
                 to: msg.sender,
                 liquidityConfigs: params.liquidityConfigs,
@@ -273,13 +277,14 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
         returns (BalanceDelta delta)
     {
         PoolId id = key.toId();
-        _checkPoolInitialized(id);
+        BinPool.State storage pool = pools[id];
+        pool.checkPoolInitialized();
 
         BinHooks.beforeBurn(key, params, hookData);
 
         uint256[] memory binIds;
         bytes32[] memory amountRemoved;
-        (delta, binIds, amountRemoved) = pools[id].burn(
+        (delta, binIds, amountRemoved) = pool.burn(
             BinPool.BurnParams({
                 from: msg.sender,
                 ids: params.ids,
@@ -307,11 +312,12 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
         returns (BalanceDelta delta, uint24 binId)
     {
         PoolId id = key.toId();
-        _checkPoolInitialized(id);
+        BinPool.State storage pool = pools[id];
+        pool.checkPoolInitialized();
 
         BinHooks.beforeDonate(key, amount0, amount1, hookData);
 
-        (delta, binId) = pools[id].donate(key.parameters.getBinStep(), amount0, amount1);
+        (delta, binId) = pool.donate(key.parameters.getBinStep(), amount0, amount1);
 
         vault.accountAppBalanceDelta(key, delta, msg.sender);
 
@@ -342,8 +348,4 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
     function _setProtocolFee(PoolId id, uint24 newProtocolFee) internal override {
         pools[id].setProtocolFee(newProtocolFee);
     }
-
-    function _checkPoolInitialized(PoolId id) internal view {
-        if (pools[id].isNotInitialized()) revert PoolNotInitialized();
-    }
 }
diff --git a/src/pool-bin/libraries/BinPool.sol b/src/pool-bin/libraries/BinPool.sol
@@ -81,13 +81,13 @@ library BinPool {
     }
 
     function setProtocolFee(State storage self, uint24 protocolFee) internal {
-        if (self.isNotInitialized()) revert PoolNotInitialized();
+        self.checkPoolInitialized();
         self.slot0.protocolFee = protocolFee;
     }
 
     /// @notice Only dynamic fee pools may update the swap fee.
     function setLPFee(State storage self, uint24 lpFee) internal {
-        if (self.isNotInitialized()) revert PoolNotInitialized();
+        self.checkPoolInitialized();
 
         self.slot0.lpFee = lpFee;
     }
@@ -524,7 +524,13 @@ library BinPool {
         (, self.level0) = TreeMath.remove(self.level0, self.level1, self.level2, binId);
     }
 
-    function isNotInitialized(State storage self) internal view returns (bool) {
-        return self.slot0.activeId == 0;
+    function checkPoolInitialized(State storage self) internal view {
+        if (self.slot0.activeId == 0) {
+            // revert PoolNotInitialized();
+            assembly ("memory-safe") {
+                mstore(0x00, 0x486aa307)
+                revert(0x1c, 0x04)
+            }
+        }
     }
 }
diff --git a/src/pool-cl/CLPoolManager.sol b/src/pool-cl/CLPoolManager.sol
@@ -130,11 +130,12 @@ contract CLPoolManager is ICLPoolManager, ProtocolFees, Extsload {
         if (paused() && params.liquidityDelta > 0) revert PoolPaused();
 
         PoolId id = key.toId();
-        _checkPoolInitialized(id);
+        CLPool.State storage pool = pools[id];
+        pool.checkPoolInitialized();
 
         CLHooks.beforeModifyLiquidity(key, params, hookData);
 
-        (delta, feeDelta) = pools[id].modifyLiquidity(
+        (delta, feeDelta) = pool.modifyLiquidity(
             CLPool.ModifyLiquidityParams({
                 owner: msg.sender,
                 tickLower: params.tickLower,
@@ -167,12 +168,13 @@ contract CLPoolManager is ICLPoolManager, ProtocolFees, Extsload {
         if (params.amountSpecified == 0) revert SwapAmountCannotBeZero();
 
         PoolId id = key.toId();
-        _checkPoolInitialized(id);
+        CLPool.State storage pool = pools[id];
+        pool.checkPoolInitialized();
 
         (int256 amountToSwap, BeforeSwapDelta beforeSwapDelta, uint24 lpFeeOverride) =
             CLHooks.beforeSwap(key, params, hookData);
         CLPool.SwapState memory state;
-        (delta, state) = pools[id].swap(
+        (delta, state) = pool.swap(
             CLPool.SwapParams({
                 tickSpacing: key.parameters.getTickSpacing(),
                 zeroForOne: params.zeroForOne,
@@ -221,12 +223,13 @@ contract CLPoolManager is ICLPoolManager, ProtocolFees, Extsload {
         returns (BalanceDelta delta)
     {
         PoolId id = key.toId();
-        _checkPoolInitialized(id);
+        CLPool.State storage pool = pools[id];
+        pool.checkPoolInitialized();
 
         CLHooks.beforeDonate(key, amount0, amount1, hookData);
 
         int24 tick;
-        (delta, tick) = pools[id].donate(amount0, amount1);
+        (delta, tick) = pool.donate(amount0, amount1);
         vault.accountAppBalanceDelta(key, delta, msg.sender);
 
         /// @notice Make sure the first event is noted, so that later events from afterHook won't get mixed up with this one
@@ -265,10 +268,6 @@ contract CLPoolManager is ICLPoolManager, ProtocolFees, Extsload {
         pools[id].setProtocolFee(newProtocolFee);
     }
 
-    function _checkPoolInitialized(PoolId id) internal view {
-        if (pools[id].isNotInitialized()) revert PoolNotInitialized();
-    }
-
     /// @notice not accept ether
     // receive() external payable {}
     // fallback() external payable {}