fix: rounding in the protocol's favor

pkg/solidity-utils/contracts/math/BasePoolMath.sol

@@ -7,6 +7,10 @@ import { FixedPoint } from "./FixedPoint.sol";
 library BasePoolMath {
     using FixedPoint for uint256;
 
+    // For security reasons, to help ensure that for all possible "round trip" paths
+    // the caller always receives the same or fewer tokens than supplied,
+    // we have chosen the rounding direction to favor the protocol in all cases.
+
     /**
      * @notice Computes the proportional amounts of tokens to be deposited into the pool.
      * @dev This function computes the amount of each token that needs to be deposited in order to mint a specific
@@ -127,7 +131,7 @@ library BasePoolMath {
         swapFeeAmounts = new uint256[](numTokens);
 
         // Loop through each token, updating the balance with the added amount.
-        for (uint256 index = 0; index < currentBalances.length; index++) {
+        for (uint256 index = 0; index < numTokens; index++) {
             newBalances[index] = currentBalances[index] + exactAmounts[index];
         }
 
@@ -141,11 +145,13 @@ library BasePoolMath {
         uint256 invariantRatio = newInvariant.divDown(currentInvariant);
 
         // Loop through each token to apply fees if necessary.
-        for (uint256 index = 0; index < currentBalances.length; index++) {
-            // Check if the new balance is greater than the proportional balance.
-            // If so, calculate the taxable amount.
-            if (newBalances[index] > invariantRatio.mulUp(currentBalances[index])) {
-                uint256 taxableAmount = newBalances[index] - invariantRatio.mulUp(currentBalances[index]);
+        for (uint256 index = 0; index < numTokens; index++) {
+            // Check if the new balance is greater than the equivalent proportional balance.
+            // If so, calculate the taxable amount, rounding in favor of the protocol.
+            // We round the second term down to subtract less and get a higher `taxableAmount`,
+            // which charges higher swap fees, reducing the amount of BPT that will be minted.
+            if (newBalances[index] > invariantRatio.mulDown(currentBalances[index])) {
+                uint256 taxableAmount = newBalances[index] - invariantRatio.mulDown(currentBalances[index]);
                 // Calculate fee amount
                 swapFeeAmounts[index] = taxableAmount.mulUp(swapFeePercentage);
                 // Subtract the fee from the new balance.
@@ -159,7 +165,7 @@ library BasePoolMath {
 
         // Calculate the amount of BPT to mint. This is done by multiplying the
         // total supply with the ratio of the change in invariant.
-        // mulDown/divDown minize amount of pool tokens to mint.
+        // mulDown/divDown minimizes the amount of pool tokens to mint for security reasons.
         bptAmountOut = totalSupply.mulDown((invariantWithFeesApplied - currentInvariant).divDown(currentInvariant));
     }
 
@@ -191,15 +197,20 @@ library BasePoolMath {
         uint256 newSupply = exactBptAmountOut + totalSupply;
         // Calculate the initial amount of the input token needed for the desired amount of BPT out
         // "divUp" leads to a higher "newBalance," which in turn results in a larger "amountIn."
-        // This leads to receiving more tokens for the same amount of BTP minted.
+        // This leads to receiving more tokens for the same amount of BPT minted.
         uint256 newBalance = computeBalance(currentBalances, tokenInIndex, newSupply.divUp(totalSupply));
+
+        // Compute the amount to be deposited into the pool.
         uint256 amountIn = newBalance - currentBalances[tokenInIndex];
 
-        // Calculate the taxable amount, which is the difference
-        // between the actual amount in and the non-taxable balance
-        uint256 nonTaxableBalance = newSupply.mulUp(currentBalances[tokenInIndex]).divDown(totalSupply);
+        // Calculate the non-taxable amount, which is the new balance proportionate to the BPT minted.
+        // Round the `nonTaxableBalance` down to favor the protocol by increasing the taxable amount, which charges
+        // higher swap fees, ultimately increasing the amount of `tokenIn` that will be transferred from the caller.
+        uint256 nonTaxableBalance = newSupply.mulDown(currentBalances[tokenInIndex]).divDown(totalSupply);
 
-        uint256 taxableAmount = (amountIn + currentBalances[tokenInIndex]) - nonTaxableBalance;
+        // Calculate the taxable amount, which is the difference
+        // between the actual new balance and the non-taxable balance
+        uint256 taxableAmount = newBalance - nonTaxableBalance;
 
         // Calculate the swap fee based on the taxable amount and the swap fee percentage
         uint256 fee = taxableAmount.divUp(swapFeePercentage.complement()) - taxableAmount;
@@ -237,22 +248,27 @@ library BasePoolMath {
         uint256[] memory newBalances = new uint256[](numTokens);
 
         // Copy currentBalances to newBalances
-        // TODO: Optimize with assembly
         for (uint256 index = 0; index < currentBalances.length; index++) {
             newBalances[index] = currentBalances[index];
         }
+
         // Update the balance of tokenOutIndex with exactAmountOut.
         newBalances[tokenOutIndex] = newBalances[tokenOutIndex] - exactAmountOut;
 
-        // Calculate the invariant using the current balances.
+        // Calculate the invariant using the current balances (before the removal).
         uint256 currentInvariant = computeInvariant(currentBalances);
 
-        // Calculate the new invariant ratio by dividing the new invariant by the current invariant.
-        // Calculate the taxable amount by subtracting the new balance from the equivalent proportional balance.
+        // Calculate the new invariant using the new balances (after the removal).
+        // Calculate the new invariant ratio by dividing the new invariant by the old invariant.
+        // Calculate the new proportional balance by multiplying the new invariant ratio by the current balance.
+        // Calculate the taxable amount by subtracting the new balance from the equivalent proportional balance,
+        // rounding in favor of the protocol. We round the first term up to subtract from more and get a higher
+        // `taxableAmount`, which charges higher swap fees, augmenting the amount of BPT that will be burned.
         uint256 taxableAmount = computeInvariant(newBalances).divUp(currentInvariant).mulUp(
             currentBalances[tokenOutIndex]
         ) - newBalances[tokenOutIndex];
 
+        // Calculate the swap fee based on the taxable amount and the swap fee percentage
         uint256 fee = taxableAmount.divUp(swapFeePercentage.complement()) - taxableAmount;
 
         // Update new balances array with a fee
@@ -265,8 +281,10 @@ library BasePoolMath {
         swapFeeAmounts = new uint256[](numTokens);
         swapFeeAmounts[tokenOutIndex] = fee;
 
-        // mulUp/divDown maximize the amount of tokens burned for the security reasons
-        bptAmountIn = totalSupply.mulUp(currentInvariant - invariantWithFeesApplied).divDown(currentInvariant);
+        // Calculate the amount of BPT to burn. This is done by multiplying the
+        // total supply with the ratio of the change in invariant.
+        // mulUp/divUp maximizes the amount of pool tokens to burn for security reasons.
+        bptAmountIn = totalSupply.mulUp(currentInvariant - invariantWithFeesApplied).divUp(currentInvariant);
     }
 
     /**
@@ -294,17 +312,19 @@ library BasePoolMath {
         uint256 newSupply = totalSupply - exactBptAmountIn;
         // Calculate the new balance of the output token after the BPT burn.
         // "divUp" leads to a higher "newBalance," which in turn results in a lower "amountOut."
-        // This leads to giving less tokens for the same amount of BTP burned.
+        // This leads to giving less tokens for the same amount of BPT burned.
         uint256 newBalance = computeBalance(currentBalances, tokenOutIndex, newSupply.divUp(totalSupply));
 
         // Compute the amount to be withdrawn from the pool.
         uint256 amountOut = currentBalances[tokenOutIndex] - newBalance;
 
-        // Calculate the non-taxable balance proportionate to the BPT burnt.
-        uint256 nonTaxableBalance = newSupply.mulUp(currentBalances[tokenOutIndex]).divDown(totalSupply);
+        // Calculate the new balance proportionate to the BPT burnt.
+        // Round the `newBalanceBeforeTax` up to favor the protocol by increasing the taxable amount, which charges
+        // higher swap fees, ultimately decreasing the amount of `tokenOut` that will be transferred to the caller.
+        uint256 newBalanceBeforeTax = newSupply.mulUp(currentBalances[tokenOutIndex]).divUp(totalSupply);
 
-        // Compute the taxable amount: the difference between the non-taxable balance and actual withdrawal.
-        uint256 taxableAmount = nonTaxableBalance - newBalance;
+        // Compute the taxable amount: the difference between the new proportional and disproportional balances.
+        uint256 taxableAmount = newBalanceBeforeTax - newBalance;
 
         // Calculate the swap fee on the taxable amount.
         uint256 fee = taxableAmount.mulUp(swapFeePercentage);

pkg/vault/.forge-snapshots/vaultRemoveLiquiditySingleTokenExactOut.snap

@@ -1 +1 @@
-132100
+132098