Record swap fee amount in Recovery Mode (#1096)

Co-authored-by: Juan Ignacio Ubeira <[email protected]>

pkg/pool-stable/test/gas/.hardhat-snapshots/[StablePool - ERC4626 - BatchRouter] swapExactOut - with buffer liquidity - warm slots

@@ -1 +1 @@
-257.8k
+257.7k

pkg/pool-stable/test/gas/.hardhat-snapshots/[StablePool - Standard - BatchRouter] add liquidity unbalanced using swapExactIn - warm slots

@@ -1 +1 @@
-196.4k
+196.3k

pkg/pool-stable/test/gas/.hardhat-snapshots/[StablePool - Standard] add liquidity unbalanced - warm slots

@@ -1 +1 @@
-214.6k
+214.5k

pkg/pool-stable/test/gas/.hardhat-snapshots/[StablePool - WithRate - BatchRouter] add liquidity unbalanced using swapExactIn - warm slots

@@ -1 +1 @@
-217.8k
+217.7k

pkg/pool-stable/test/gas/.hardhat-snapshots/[StablePool - WithRate - BatchRouter] add liquidity using swapExactOur - warm slots

@@ -1 +1 @@
-202.0k
+201.9k

pkg/pool-stable/test/gas/.hardhat-snapshots/[StablePool - WithRate - BatchRouter] remove liquidity using swapExactIn - warm slots

@@ -1 +1 @@
-215.0k
+214.9k

pkg/pool-stable/test/gas/.hardhat-snapshots/[StablePool - WithRate - BatchRouter] remove liquidity using swapExactOut - warm slots

@@ -1 +1 @@
-234.9k
+234.8k

pkg/pool-stable/test/gas/.hardhat-snapshots/[StablePool - WithRate] remove liquidity proportional

@@ -1 +1 @@
-222.3k
+222.2k

pkg/pool-stable/test/gas/.hardhat-snapshots/[StablePool - WithRate] remove liquidity single token exact out - warm slots

@@ -1 +1 @@
-198.1k
+198.0k

pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - WithRate - BatchRouter] remove liquidity using swapExactIn - warm slots

@@ -1 +1 @@
-201.6k
+201.5k

pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - WithRate] add liquidity single token exact out - warm slots

@@ -1 +1 @@
-179.9k
+179.8k

pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - WithRate] remove liquidity single token exact in - warm slots

@@ -1 +1 @@
-174.4k
+174.3k

pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - WithRate] remove liquidity single token exact out - warm slots

@@ -1 +1 @@
-197.7k
+197.6k

pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool] donation

@@ -0,0 +1 @@
+178.0k

pkg/vault/contracts/Vault.sol

@@ -1039,33 +1039,38 @@ contract Vault is IVaultMain, VaultCommon, Proxy {
         uint256 index
     ) internal returns (uint256 totalSwapFeeAmountRaw, uint256 aggregateSwapFeeAmountRaw) {
         // If totalSwapFeeAmountScaled18 equals zero, no need to charge anything.
-        if (totalSwapFeeAmountScaled18 > 0 && poolData.poolConfigBits.isPoolInRecoveryMode() == false) {
+        if (totalSwapFeeAmountScaled18 > 0) {
             // The total swap fee does not go into the pool; amountIn does, and the raw fee at this point does not
             // modify it. Given that all of the fee may belong to the pool creator (i.e. outside pool balances),
             // we round down to protect the invariant.
+
             totalSwapFeeAmountRaw = totalSwapFeeAmountScaled18.toRawUndoRateRoundDown(
                 poolData.decimalScalingFactors[index],
                 poolData.tokenRates[index]
             );
 
-            uint256 aggregateSwapFeePercentage = poolData.poolConfigBits.getAggregateSwapFeePercentage();
+            // Aggregate fees are not charged in Recovery Mode, but we still calculate and return the raw total swap
+            // fee above for off-chain reporting purposes.
+            if (poolData.poolConfigBits.isPoolInRecoveryMode() == false) {
+                uint256 aggregateSwapFeePercentage = poolData.poolConfigBits.getAggregateSwapFeePercentage();
 
-            // We have already calculated raw total fees rounding up.
-            // Total fees = LP fees + aggregate fees, so by rounding aggregate fees down we round the fee split in
-            // the LPs' favor, in turn increasing token balances and the pool invariant.
-            aggregateSwapFeeAmountRaw = totalSwapFeeAmountRaw.mulDown(aggregateSwapFeePercentage);
+                // We have already calculated raw total fees rounding up.
+                // Total fees = LP fees + aggregate fees, so by rounding aggregate fees down we round the fee split in
+                // the LPs' favor, in turn increasing token balances and the pool invariant.
+                aggregateSwapFeeAmountRaw = totalSwapFeeAmountRaw.mulDown(aggregateSwapFeePercentage);
 
-            // Ensure we can never charge more than the total swap fee.
-            if (aggregateSwapFeeAmountRaw > totalSwapFeeAmountRaw) {
-                revert ProtocolFeesExceedTotalCollected();
-            }
+                // Ensure we can never charge more than the total swap fee.
+                if (aggregateSwapFeeAmountRaw > totalSwapFeeAmountRaw) {
+                    revert ProtocolFeesExceedTotalCollected();
+                }
 
-            // Both Swap and Yield fees are stored together in a PackedTokenBalance.
-            // We have designated "Raw" the derived half for Swap fee storage.
-            bytes32 currentPackedBalance = _aggregateFeeAmounts[pool][token];
-            _aggregateFeeAmounts[pool][token] = currentPackedBalance.setBalanceRaw(
-                currentPackedBalance.getBalanceRaw() + aggregateSwapFeeAmountRaw
-            );
+                // Both Swap and Yield fees are stored together in a PackedTokenBalance.
+                // We have designated "Raw" the derived half for Swap fee storage.
+                bytes32 currentPackedBalance = _aggregateFeeAmounts[pool][token];
+                _aggregateFeeAmounts[pool][token] = currentPackedBalance.setBalanceRaw(
+                    currentPackedBalance.getBalanceRaw() + aggregateSwapFeeAmountRaw
+                );
+            }
         }
     }
 

pkg/vault/test/.contract-sizes/Vault

@@ -1,2 +1,2 @@
-Bytecode	23.569
-InitCode	24.978
+Bytecode	23.566
+InitCode	24.975

pkg/vault/test/foundry/VaultLiquidity.t.sol

@@ -7,6 +7,7 @@ import "forge-std/Test.sol";
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 
 import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol";
+import { IVaultEvents } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultEvents.sol";
 import { PoolConfig } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol";
 
 import { ArrayHelpers } from "@balancer-labs/v3-solidity-utils/contracts/test/ArrayHelpers.sol";
@@ -446,6 +447,74 @@ contract VaultLiquidityTest is BaseVaultTest {
         assertEq(balancesAfter.userBpt, 0, "Roundtrip - User BPT balance after");
     }
 
+    function testSwapFeesInEventRemoveLiquidityInRecovery() public {
+        setSwapFeePercentage(swapFeePercentage);
+        vault.manualEnableRecoveryMode(pool);
+
+        uint256 totalSupplyBefore = IERC20(pool).totalSupply();
+        uint256 bptAmountIn = defaultAmount;
+
+        uint256 snapshotId = vm.snapshot();
+
+        vm.prank(lp);
+        uint256 amountOut = router.removeLiquiditySingleTokenExactIn(
+            pool,
+            bptAmountIn,
+            dai,
+            defaultAmount / 10,
+            false,
+            bytes("")
+        );
+
+        vm.revertTo(snapshotId);
+
+        uint256 swapFeeAmountDai = 5e18;
+        int256[] memory deltas = new int256[](2);
+        deltas[daiIdx] = -int256(amountOut);
+
+        // Exact values for swap fees are tested elsewhere; we only want to prove they are not 0 here.
+        uint256[] memory swapFeeAmounts = new uint256[](2);
+        swapFeeAmounts[daiIdx] = swapFeeAmountDai;
+
+        // Fee should be non-zero, even in RecoveryMode
+        vm.expectEmit();
+        emit IVaultEvents.PoolBalanceChanged(pool, lp, totalSupplyBefore - bptAmountIn, deltas, swapFeeAmounts);
+        vm.prank(lp);
+        router.removeLiquiditySingleTokenExactIn(pool, bptAmountIn, dai, defaultAmount / 10, false, bytes(""));
+    }
+
+    function testSwapFeesInEventAddLiquidityInRecovery() public {
+        setSwapFeePercentage(swapFeePercentage);
+        vault.manualEnableRecoveryMode(pool);
+
+        uint256 totalSupplyBefore = IERC20(pool).totalSupply();
+        uint256[] memory amountsIn = [defaultAmount, defaultAmount].toMemoryArray();
+
+        uint256 snapshotId = vm.snapshot();
+
+        vm.prank(alice);
+        uint256 bptAmountOut = router.addLiquidityUnbalanced(pool, amountsIn, 0, false, bytes(""));
+
+        vm.revertTo(snapshotId);
+
+        int256[] memory deltas = new int256[](2);
+        deltas[daiIdx] = int256(amountsIn[daiIdx]);
+        deltas[usdcIdx] = int256(amountsIn[usdcIdx]);
+
+        // Exact values for swap fees are tested elsewhere; we only want to prove they are not 0 here.
+        // The add is proportional except for rounding errors so the swap fees here are negligible (but not 0).
+        uint256[] memory swapFeeAmounts = new uint256[](2);
+        swapFeeAmounts[daiIdx] = 10;
+        swapFeeAmounts[usdcIdx] = 10;
+
+        // Fee should be non-zero, even in RecoveryMode
+        vm.expectEmit();
+        emit IVaultEvents.PoolBalanceChanged(pool, alice, totalSupplyBefore + bptAmountOut, deltas, swapFeeAmounts);
+
+        vm.prank(alice);
+        router.addLiquidityUnbalanced(pool, amountsIn, 0, false, bytes(""));
+    }
+
     // Utils
 
     function assertAddLiquidity(function() returns (uint256[] memory, uint256) testFunc) internal {

pkg/vault/test/foundry/VaultSwap.t.sol

@@ -234,6 +234,36 @@ contract VaultSwapTest is BaseVaultTest {
         );
     }
 
+    function testSwapEventExactInRecovery() public {
+        setSwapFeePercentage(swapFeePercentage);
+
+        vault.manualEnableRecoveryMode(pool);
+
+        // Fee should be non-zero, even in RecoveryMode
+        vm.expectEmit();
+        emit IVaultEvents.Swap(
+            pool,
+            usdc,
+            dai,
+            defaultAmount,
+            defaultAmount - swapFeeExactIn,
+            swapFeePercentage,
+            defaultAmount.mulDown(swapFeePercentage)
+        );
+
+        vm.prank(alice);
+        router.swapSingleTokenExactIn(
+            pool,
+            usdc,
+            dai,
+            defaultAmount,
+            defaultAmount - swapFeeExactIn,
+            MAX_UINT256,
+            false,
+            bytes("")
+        );
+    }
+
     function testSwapEventExactOut() public {
         setSwapFeePercentage(swapFeePercentage);
 

pkg/vault/test/foundry/unit/VaultUnit.t.sol

@@ -38,8 +38,8 @@ contract VaultUnitTest is BaseTest, VaultContractsDeployer {
 
     address pool = address(0x1234);
     uint256 amountGivenRaw = 1 ether;
-    uint256[] decimalScalingFactors = [uint256(1e18), 1e18];
-    uint256[] tokenRates = [uint256(1e18), 2e18];
+    uint256[] decimalScalingFactors = [1e18, 1e18];
+    uint256[] tokenRates = [1e18, 2e18];
 
     function setUp() public virtual override {
         BaseTest.setUp();
@@ -124,9 +124,9 @@ contract VaultUnitTest is BaseTest, VaultContractsDeployer {
         poolData.poolConfigBits = poolData.poolConfigBits.setPoolInRecoveryMode(true);
 
         (uint256 totalSwapFeeAmountRaw, uint256 aggregateSwapFeeAmountRaw) = vault
-            .manualComputeAndChargeAggregateSwapFees(poolData, 1e18, pool, dai, 0);
+            .manualComputeAndChargeAggregateSwapFees(poolData, 2e18, pool, dai, 0);
 
-        assertEq(totalSwapFeeAmountRaw, 0, "Unexpected totalSwapFeeAmountRaw");
+        assertEq(totalSwapFeeAmountRaw, 2, "Unexpected totalSwapFeeAmountRaw");
         assertEq(aggregateSwapFeeAmountRaw, 0, "Unexpected aggregateSwapFeeAmountRaw");
         assertEq(vault.getAggregateSwapFeeAmount(pool, dai), 0, "Unexpected protocol fees in storage");
     }

pkg/vault/test/gas/.hardhat-snapshots/[PoolMock - Standard - BatchRouter] add liquidity using swapExactOur - warm slots

@@ -1 +1 @@
-167.3k
+167.2k

pkg/vault/test/gas/.hardhat-snapshots/[PoolMock - Standard - BatchRouter] remove liquidity using swapExactOut - warm slots

@@ -1 +1 @@
-196.6k
+196.5k

pkg/vault/test/gas/.hardhat-snapshots/[PoolMock - WithRate - BatchRouter] remove liquidity using swapExactIn - warm slots

@@ -1 +1 @@
-201.7k
+201.6k

pkg/vault/test/gas/.hardhat-snapshots/[PoolMockWithHooks - Standard - BatchRouter] add liquidity unbalanced using swapExactIn - warm slots

@@ -1 +1 @@
-190.9k
+190.8k

pkg/vault/test/gas/.hardhat-snapshots/[PoolMockWithHooks - Standard - BatchRouter] remove liquidity using swapExactIn - warm slots

@@ -1 +1 @@
-198.4k
+198.3k

pkg/vault/test/gas/.hardhat-snapshots/[PoolMockWithHooks - Standard] add liquidity proportional

@@ -1 +1 @@
-197.3k
+197.2k

pkg/vault/test/gas/.hardhat-snapshots/[PoolMockWithHooks - Standard] add liquidity single token exact out - warm slots

@@ -1 +1 @@
-176.5k
+176.4k

pkg/vault/test/gas/.hardhat-snapshots/[PoolMockWithHooks - Standard] remove liquidity single token exact in - warm slots

@@ -1 +1 @@
-171.2k
+171.1k

pkg/vault/test/gas/.hardhat-snapshots/[PoolMockWithHooks - WithRate - BatchRouter] add liquidity unbalanced using swapExactIn - warm slots

@@ -1 +1 @@
-214.2k
+214.1k

pkg/vault/test/gas/.hardhat-snapshots/[PoolMockWithHooks - WithRate] add liquidity proportional

@@ -1 +1 @@
-254.3k
+254.2k

pkg/vault/test/gas/.hardhat-snapshots/[PoolMockWithHooks - WithRate] add liquidity single token exact out - warm slots

@@ -1 +1 @@
-199.8k
+199.7k

pkg/vault/test/gas/.hardhat-snapshots/[PoolMockWithHooks - WithRate] remove liquidity proportional

@@ -1 +1 @@
-242.2k
+242.1k

pkg/vault/test/gas/.hardhat-snapshots/[PoolMockWithHooks - WithRate] remove liquidity single token exact out - warm slots

@@ -1 +1 @@
-200.9k
+200.8k

pkg/vault/test/gas/.hardhat-snapshots/[PoolMock] donation

@@ -0,0 +1 @@
+178.0k