[Discuss]feat/refactor account delta for app (#171)

* feat: Refactor accountAppBalanceDelta in vault Remove PoolKey in accountAppBalanceDelta parameters. Just pass currency0 and currency1 , will save some gas * feat: Remove PoolKey in vault * feat: Optimize logic * feat: Optimize _accountDeltaForApp Remove parameter app , will save some gas * feat: Remove app from _accountDeltaForApp in MockVault * fix: Update * feat: Add comments * fix: Remove unused import files
pancakeswap · Sep 12, 2024 · 905a96e · 905a96e
1 parent 098f944
commit 905a96e
Show file tree

Hide file tree

Showing 34 changed files with 44 additions and 46 deletions.
diff --git a/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap
@@ -1 +1 @@
-178384
+178372
diff --git a/.forge-snapshots/BinHookTest#testDonateSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testDonateSucceedsWithHook.snap
@@ -1 +1 @@
-188536
+188524
diff --git a/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap
@@ -1 +1 @@
-311585
+311573
diff --git a/.forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap
@@ -1 +1 @@
-189835
+189823
diff --git a/.forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap b/.forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap
@@ -1 +1 @@
-133896
+133886
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap b/.forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap
@@ -1 +1 @@
-142709
+142697
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap b/.forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap
@@ -1 +1 @@
-290054
+290044
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap b/.forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap
@@ -1 +1 @@
-127070
+127060
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasDonate.snap b/.forge-snapshots/BinPoolManagerTest#testGasDonate.snap
@@ -1 +1 @@
-118808
+118796
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap
@@ -1 +1 @@
-970989
+970977
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap
@@ -1 +1 @@
-330300
+330288
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap
@@ -1 +1 @@
-337901
+337889
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap
@@ -1 +1 @@
-140451
+140439
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasSwapMultipleBins.snap b/.forge-snapshots/BinPoolManagerTest#testGasSwapMultipleBins.snap
@@ -1 +1 @@
-173359
+173347
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasSwapOverBigBinIdGate.snap b/.forge-snapshots/BinPoolManagerTest#testGasSwapOverBigBinIdGate.snap
@@ -1 +1 @@
-179388
+179376
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasSwapSingleBin.snap b/.forge-snapshots/BinPoolManagerTest#testGasSwapSingleBin.snap
@@ -1 +1 @@
-133390
+133378
diff --git a/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap b/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap
@@ -1 +1 @@
-304858
+304846
diff --git a/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap b/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap
@@ -1 +1 @@
-347647
+347635
diff --git a/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap b/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap
@@ -1 +1 @@
-163118
+163106
diff --git a/.forge-snapshots/CLPoolManagerTest#addLiquidity_nativeToken.snap b/.forge-snapshots/CLPoolManagerTest#addLiquidity_nativeToken.snap
@@ -1 +1 @@
-238402
+238392
diff --git a/.forge-snapshots/CLPoolManagerTest#donateBothTokens.snap b/.forge-snapshots/CLPoolManagerTest#donateBothTokens.snap
@@ -1 +1 @@
-163373
+163361
diff --git a/.forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap b/.forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap
@@ -1 +1 @@
-108318
+108308
diff --git a/.forge-snapshots/CLPoolManagerTest#removeLiquidity_toNonEmpty.snap b/.forge-snapshots/CLPoolManagerTest#removeLiquidity_toNonEmpty.snap
@@ -1 +1 @@
-114876
+114866
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_againstLiquidity.snap b/.forge-snapshots/CLPoolManagerTest#swap_againstLiquidity.snap
@@ -1 +1 @@
-131112
+131100
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_leaveSurplusTokenInVault.snap b/.forge-snapshots/CLPoolManagerTest#swap_leaveSurplusTokenInVault.snap
@@ -1 +1 @@
-163690
+163678
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap b/.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap
@@ -1 +1 @@
-149129
+149117
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_simple.snap b/.forge-snapshots/CLPoolManagerTest#swap_simple.snap
@@ -1 +1 @@
-71720
+71712
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_useSurplusTokenAsInput.snap b/.forge-snapshots/CLPoolManagerTest#swap_useSurplusTokenAsInput.snap
@@ -1 +1 @@
-143365
+143353
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_withHooks.snap b/.forge-snapshots/CLPoolManagerTest#swap_withHooks.snap
@@ -1 +1 @@
-87992
+87984
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_withNative.snap b/.forge-snapshots/CLPoolManagerTest#swap_withNative.snap
@@ -1 +1 @@
-71723
+71715
diff --git a/.forge-snapshots/VaultBytecodeSize.snap b/.forge-snapshots/VaultBytecodeSize.snap
@@ -1 +1 @@
-7702
+7651
diff --git a/src/Vault.sol b/src/Vault.sol
@@ -83,8 +83,8 @@ contract Vault is IVault, VaultToken, Ownable {
         int128 delta1 = delta.amount1();
 
         // keep track of the balance on app level
-        _accountDeltaForApp(msg.sender, currency0, delta0);
-        _accountDeltaForApp(msg.sender, currency1, delta1);
+        _accountDeltaForApp(currency0, delta0);
+        _accountDeltaForApp(currency1, delta1);
 
         // keep track of the balance on vault level
         SettlementGuard.accountDelta(settler, currency0, delta0);
@@ -98,7 +98,7 @@ contract Vault is IVault, VaultToken, Ownable {
         isLocked
         onlyRegisteredApp
     {
-        _accountDeltaForApp(msg.sender, currency, delta);
+        _accountDeltaForApp(currency, delta);
         SettlementGuard.accountDelta(settler, currency, delta);
     }
 
@@ -166,15 +166,16 @@ contract Vault is IVault, VaultToken, Ownable {
         return VaultReserve.getVaultReserve();
     }
 
-    function _accountDeltaForApp(address app, Currency currency, int128 delta) internal {
+    function _accountDeltaForApp(Currency currency, int128 delta) internal {
         if (delta == 0) return;
 
+        /// @dev optimization: msg.sender will always be app address, verification should be done on caller address
         if (delta >= 0) {
             /// @dev arithmetic underflow make sure trader can't withdraw too much from app
-            reservesOfApp[app][currency] -= uint128(delta);
+            reservesOfApp[msg.sender][currency] -= uint128(delta);
         } else {
             /// @dev arithmetic overflow make sure trader won't deposit too much into app
-            reservesOfApp[app][currency] += uint128(-delta);
+            reservesOfApp[msg.sender][currency] += uint128(-delta);
         }
     }
 

diff --git a/src/interfaces/IVault.sol b/src/interfaces/IVault.sol
@@ -2,8 +2,6 @@
 pragma solidity ^0.8.0;
 
 import {Currency} from "../types/Currency.sol";
-import {PoolId} from "../types/PoolId.sol";
-import {PoolKey} from "../types/PoolKey.sol";
 import {BalanceDelta} from "../types/BalanceDelta.sol";
 import {IVaultToken} from "./IVaultToken.sol";
 

diff --git a/src/test/MockVault.sol b/src/test/MockVault.sol
@@ -36,25 +36,24 @@ contract MockVault {
                 revert InvalidPoolKey();
             }
         }
-
-        _accountDeltaForApp(msg.sender, currency0, delta.amount0());
-        _accountDeltaForApp(msg.sender, currency1, delta.amount1());
+        _accountDeltaForApp(currency0, delta.amount0());
+        _accountDeltaForApp(currency1, delta.amount1());
     }
 
-    function _accountDeltaForApp(address app, Currency currency, int128 delta) internal {
+    function _accountDeltaForApp(Currency currency, int128 delta) internal {
         if (delta == 0) return;
 
         if (delta >= 0) {
             /// @dev arithmetic underflow make sure trader can't withdraw too much from app
-            reservesOfApp[app][currency] -= uint128(delta);
+            reservesOfApp[msg.sender][currency] -= uint128(delta);
         } else {
             /// @dev arithmetic overflow make sure trader won't deposit too much into app
-            reservesOfApp[app][currency] += uint128(-delta);
+            reservesOfApp[msg.sender][currency] += uint128(-delta);
         }
     }
 
     function collectFee(Currency currency, uint256 amount, address recipient) external {
-        _accountDeltaForApp(msg.sender, currency, -amount.toInt128());
+        _accountDeltaForApp(currency, -amount.toInt128());
         currency.transfer(recipient, amount);
     }
 }