Basin - pfapostol's results

Lines of code

https://github.com/code-423n4/2023-07-basin/blob/9403cf973e95ef7219622dbbe2a08396af90b64c/src/functions/ProportionalLPToken2.sol#L14 https://github.com/code-423n4/2023-07-basin/blob/9403cf973e95ef7219622dbbe2a08396af90b64c/src/functions/ConstantProduct2.sol#L52

Vulnerability details

Impact

liquidity providers and swap users can lose their tokens without being able to get them back. Also, liquidity providers can use removeLiquidityOneToken to break liquidity calculations, since the protocol expects totalSupply to depend on the first two tokens, the presence of other tokens will lead to invalid liquidity calculation, tokens are blocked in the protocol.

Severity:

I think the severity of this Issue can be debatable, so I'll explain my assessment:

1. I can't consider it High as the project itself is just a framework and the user has to manually check if certain wells and well functions are safe to use.
2. But I don't consider it Low because:
   1. ConstantProduct2 is a function created by Beanstalk and will be more trusted by users than any arbitrary function. Thus, an attacker can easily create a malicious multi-token well with the same "trusted well function" and lure the user to lose their funds.
   2. Also in the context of the ConstantProduct2 and ProportionalLPToken2 functions, there is no need to accept more than 2 tokens, limiting the number of tokens to 2 will not break any functionality, but allowing the function to accept more than 2 tokens will lead to blocking of funds and a protocol miscalculation.

So functions design itself can be cause of loss of funds.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;

import {console} from "forge-std/console.sol";
import {Test} from "forge-std/Test.sol";
import {ConstantProduct2} from "src/functions/ConstantProduct2.sol";
import {Well} from "src/Well.sol";
import {Aquifer} from "src/Aquifer.sol";
import {MultiFlowPump} from "src/pumps/MultiFlowPump.sol";
import {SafeERC20, IERC20} from "oz/token/ERC20/utils/SafeERC20.sol";
import {IWellFunction} from "src/interfaces/IWellFunction.sol";
import {Call} from "src/interfaces/IWell.sol";
import {IPump} from "src/interfaces/pumps/IPump.sol";
import {MockToken} from "mocks/tokens/MockToken.sol";
import {LibContractInfo} from "src/libraries/LibContractInfo.sol";
import {LibWellConstructor} from "src/libraries/LibWellConstructor.sol";
import {MultiFlowPump, ABDKMathQuad} from "src/pumps/MultiFlowPump.sol";
import {simCapReserve50Percent, from18, to18} from "test/pumps/PumpHelpers.sol";

/// @dev Tests the {ConstantProduct2} Well function directly.
contract ConstantProductMoreThan2Test is Test {
    using SafeERC20 for IERC20;
    using ABDKMathQuad for bytes16;

    Well well;
    IERC20[] tokens = new IERC20[](5);

    function setUp() public {
        tokens[0] = new MockToken("token1", "tkn1", 18);
        tokens[1] = new MockToken("token2", "tkn2", 18);
        tokens[2] = new MockToken("token3", "tkn3", 18);
        tokens[3] = new MockToken("token4", "tkn4", 18);
        tokens[4] = new MockToken("token5", "tkn5", 18);

        address aquifer = address(new Aquifer());

        // Well Function
        IWellFunction cp2 = new ConstantProduct2();
        Call memory wellFunction = Call(address(cp2), new bytes(0));

        // Pump
        IPump mockPump = new MultiFlowPump(
            from18(0.5e18), // cap reserves if changed +/- 50% per block
            from18(0.5e18), // cap reserves if changed +/- 50% per block
            12, // EVM block time
            from18(0.9994445987e18) // geometric EMA constant
        );
        Call[] memory pumps = new Call[](1);
        pumps[0] = Call(address(mockPump), new bytes(0));

        // Well implementation
        address wellImplementation = address(new Well());

        //bore well
        well = encodeAndBoreWell(aquifer, wellImplementation, tokens, wellFunction, pumps, bytes32(0));

        console.log("Deployed CP2 at address: ", address(cp2));
        console.log("Deployed Pump at address: ", address(pumps[0].target));
    }

    function test_intialized() public {
        console.log(address(well.tokens()[4]));
    }

    function test_PoC() public {
        address from = address(this);
        uint256 amount = 1000e18;

        for (uint256 i; i < tokens.length; i++) {
            MockToken(address(tokens[i])).mint(from, amount);
        }
        for (uint256 i; i < tokens.length; i++) {
            tokens[i].approve(address(well), type(uint256).max);
        }

        uint256[] memory amounts = new uint256[](tokens.length);
        for (uint256 i; i < tokens.length; i++) {
            amounts[i] = amount;
        }

        well.addLiquidity(amounts, 0, from, type(uint256).max);
        console.log("Liquidity added");
        console.log("   Liquidity:", well.balanceOf(address(this)));
        console.log("   Total Supply:", well.totalSupply());
        console.log("");
        console.log("");

        uint256[] memory reserves;
        console.log("BEFORE swap");
        console.log("   RESERVES");
        reserves = well.getReserves();
        for (uint256 i = 0; i < reserves.length; ++i) {
            console.log("       reserve", i, reserves[i]);
        }
        console.log("   BALANCES");
        for (uint256 i = 0; i < reserves.length; ++i) {
            console.log("       token", i, tokens[i].balanceOf(address(well)));
        }

        MockToken(address(tokens[3])).mint(from, 100e18);
        console.log("   USER");
        console.log("       token3", tokens[3].balanceOf(address(this)));
        console.log("       token4", tokens[4].balanceOf(address(this)));

        console.log("");
        console.log("");
        console.log("AFTER swap");
        well.swapFrom(tokens[3], tokens[4], 100e18, 0, address(this), type(uint256).max);

        console.log("   RESERVES");
        reserves = well.getReserves();
        for (uint256 i = 0; i < reserves.length; ++i) {
            console.log("       reserves", i, reserves[i]);
        }
        console.log("   BALANCES");
        for (uint256 i = 0; i < reserves.length; ++i) {
            console.log("       tokens", i, tokens[i].balanceOf(address(well)));
        }
        console.log("   USERS");
        console.log("       token3", tokens[3].balanceOf(address(this)));
        console.log("       token4", tokens[4].balanceOf(address(this)));

        console.log("");
        console.log("");

        for (uint256 i; i < reserves.length; i++) {
            amounts[i] = 0;
        }

        // Index out of bounds due to `_calcLPTokenUnderlying`
        vm.expectRevert();
        well.removeLiquidity(1e27, amounts, address(this), type(uint256).max);


        well.removeLiquidityOneToken(1e24, tokens[4], 0, address(this), type(uint256).max);
        well.removeLiquidityOneToken(1e24, tokens[3], 0, address(this), type(uint256).max);
        well.removeLiquidityOneToken(1e24, tokens[2], 0, address(this), type(uint256).max);
        well.removeLiquidityOneToken(1e24, tokens[1], 0, address(this), type(uint256).max);
        console.log("   User");
        for (uint256 i = 0; i < reserves.length; ++i) {
            console.log("       tokens", i, tokens[i].balanceOf(address(this)));
        }
        console.log("   Liquidity:", well.balanceOf(address(this)));
        console.log("   TotalSupply:", well.totalSupply());
    }

    function encodeAndBoreWell(
        address _aquifer,
        address _wellImplementation,
        IERC20[] memory _tokens,
        Call memory _wellFunction,
        Call[] memory _pumps,
        bytes32 _salt
    ) internal returns (Well _well) {
        (bytes memory immutableData, bytes memory initData) =
            LibWellConstructor.encodeWellDeploymentData(_aquifer, _tokens, _wellFunction, _pumps);
        _well = Well(Aquifer(_aquifer).boreWell(_wellImplementation, immutableData, initData, _salt));
    }
}

Tools Used

manual review

Recommended Mitigation Steps

In each function, add a check that ensures that the function is not called with more than two tokens.

diff --git a/src/functions/ConstantProduct2.sol b/src/functions/ConstantProduct2.sol
index 40c3b4e..c4444b0 100644
--- a/src/functions/ConstantProduct2.sol
+++ b/src/functions/ConstantProduct2.sol
@@ -49,7 +49,7 @@ contract ConstantProduct2 is ProportionalLPToken2, IBeanstalkWellFunction {
     function calcLpTokenSupply(
         uint256[] calldata reserves,
         bytes calldata
-    ) external pure override returns (uint256 lpTokenSupply) {
+    ) external pure OnlyTwo(reserves) override returns (uint256 lpTokenSupply) {
         lpTokenSupply = (reserves[0] * reserves[1] * EXP_PRECISION).sqrt();
     }

@@ -60,7 +60,7 @@ contract ConstantProduct2 is ProportionalLPToken2, IBeanstalkWellFunction {
         uint256 j,
         uint256 lpTokenSupply,
         bytes calldata
-    ) external pure override returns (uint256 reserve) {
+    ) external pure OnlyTwo(reserves) override returns (uint256 reserve) {
         // Note: potential optimization is to use unchecked math here
         reserve = lpTokenSupply ** 2;
         reserve = LibMath.roundUpDiv(reserve, reserves[j == 1 ? 0 : 1] * EXP_PRECISION);
@@ -81,7 +81,7 @@ contract ConstantProduct2 is ProportionalLPToken2, IBeanstalkWellFunction {
         uint256 j,
         uint256[] calldata ratios,
         bytes calldata
-    ) external pure override returns (uint256 reserve) {
+    ) external pure OnlyTwo(reserves) override returns (uint256 reserve) {
         uint256 i = j == 1 ? 0 : 1;
         // use 512 muldiv for last mul to avoid overflow
         reserve = (reserves[i] * reserves[j]).mulDiv(ratios[j], ratios[i]).sqrt();
@@ -94,7 +94,7 @@ contract ConstantProduct2 is ProportionalLPToken2, IBeanstalkWellFunction {
         uint256 j,
         uint256[] calldata ratios,
         bytes calldata
-    ) external pure override returns (uint256 reserve) {
+    ) external pure  OnlyTwo(reserves) override returns (uint256 reserve) {
         uint256 i = j == 1 ? 0 : 1;
         reserve = reserves[i] * ratios[j] / ratios[i];
     }
diff --git a/src/functions/ProportionalLPToken2.sol b/src/functions/ProportionalLPToken2.sol
index 748a8cf..9e678e5 100644
--- a/src/functions/ProportionalLPToken2.sol
+++ b/src/functions/ProportionalLPToken2.sol
@@ -12,12 +12,21 @@ import {IWellFunction} from "src/interfaces/IWellFunction.sol";
  * recieves `s * b_i / S` of each underlying token.
  */
 abstract contract ProportionalLPToken2 is IWellFunction {
+    error InvalidTokenAmount();
+
+    modifier OnlyTwo(uint256[] calldata reserves) {
+        if (reserves.length != 2) {
+            revert InvalidTokenAmount();
+        }
+        _;
+    }
+
     function calcLPTokenUnderlying(
         uint256 lpTokenAmount,
         uint256[] calldata reserves,
         uint256 lpTokenSupply,
         bytes calldata
-    ) external pure returns (uint256[] memory underlyingAmounts) {
+    ) external pure OnlyTwo(reserves) returns (uint256[] memory underlyingAmounts) {
         underlyingAmounts = new uint256[](2);
         underlyingAmounts[0] = lpTokenAmount * reserves[0] / lpTokenSupply;
         underlyingAmounts[1] = lpTokenAmount * reserves[1] / lpTokenSupply;

Assessed type

Invalid Validation

Gas report

Foundry config setup:

diff --git a/foundry.toml b/foundry.toml
index f4080ba..3df885b 100644
--- a/foundry.toml
+++ b/foundry.toml
@@ -10,6 +10,7 @@ remappings = [
 ]
 block_number = 16826654
 block_timestamp = 1678803642
+gas_reports = ['ConstantProduct2', 'ProportionalLPToken2', 'MultiFlowPump', 'Aquifer', 'Well', 'LibBytes', 'LibBytes16', 'LibContractInfo', 'LibLastReserveBytes', 'LibWellConstructor']

 [rpc_endpoints]
 mainnet = "${MAINNET_RPC_URL}"

• Gas report
• Public function for the public varaible
  • Code difference
• Refactoring
  • Return early as soon as the indices have been found to prevent unnecessary looping through the array.
    • Code difference
  • Don't write state changes until slippage has been checked for all tokens
    • Code difference
  • In removeLiquidityImbalanced check slippage before transfering tokens
    • Code difference
  • MultiFlowPump check for reserves != 0 twice
    • Code difference

Public function for the public varaible

<a name="G-01"></a> The external function wellImplementation only calls the public state variable wellImplementations, which can be called directly without creating additional functions.

• src\Aquifer.sol

forge test --ffi --gas-report --match-contract AquiferTest

Deployment gas cost reduced by 600

Minimum functions call gas cost reduced by 9

Average functions call gas cost reduced by 9

Maximum functions call gas cost reduced by 9

Code difference

diff --git a/src/Aquifer.sol b/src/Aquifer.sol
index 7bbc539..f768676 100644
--- a/src/Aquifer.sol
+++ b/src/Aquifer.sol
@@ -22,7 +22,7 @@ contract Aquifer is IAquifer, ReentrancyGuard {

    // A mapping of Well address to the Well implementation addresses
    // Mapping gets set on Well deployment
-    mapping(address => address) wellImplementations;
+    mapping(address => address) public wellImplementations;

    constructor() ReentrancyGuard() {}

@@ -82,8 +82,4 @@ contract Aquifer is IAquifer, ReentrancyGuard {
            IWell(well).wellData()
        );
    }
-
-    function wellImplementation(address well) external view returns (address implementation) {
-        return wellImplementations[well];
-    }
}

Refactoring

Return early as soon as the indices have been found to prevent unnecessary looping through the array.

• src\Well.sol

Code difference

diff --git a/src/Well.sol b/src/Well.sol
index 64dd3cc..6035f14 100644
--- a/src/Well.sol
+++ b/src/Well.sol
@@ -743,10 +743,12 @@ contract Well is ERC20PermitUpgradeable, IWell, IWellErrors, ReentrancyGuardUpgr
                j = k;
                foundJ = true;
            }
+            if (foundI && foundJ) {
+                return (i, j);
+            }
        }

-        if (!foundI) revert InvalidTokens();
-        if (!foundJ) revert InvalidTokens();
+        revert InvalidTokens();
    }

    /**

_getIJ is an internal function so forge doesn't generate gas report for getIJ tests so I use direct output

The number of tokens and their position is not constant, so the following test is just an example, confirming that when scaling, the savings in the best case (when indices are close to the beginning of the array) outweigh the losses in the worst case (indices at the end of the array). An average result of 5000 runs with random indexes is always more efficient for new code.

Command: forge test --gas-report --ffi --match-path "test/Well.IJ.t.sol" --fuzz-runs 5000

<details> <summary>OLD code perfomance</summary>

```
[PASS] test_best_case() (gas: 380721)
[PASS] test_search(uint256,uint256) (runs: 5000, μ: 390201, ~: 390457)
[PASS] test_worst_case() (gas: 380745)
```

</details> <details> <summary>NEW code perfomance</summary>

```
[PASS] test_best_case() (gas: 332253)
[PASS] test_search(uint256,uint256) (runs: 5000, μ: 380115, ~: 384689)
[PASS] test_worst_case() (gas: 386662)
```

</details> <details> <summary>test/Well.IJ.t.sol code</summary>

```solidity
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.17;

import {Test, console} from "forge-std/Test.sol";
import {Well, IERC20} from "test/TestHelper.sol";

contract WellSearchTest is Well, Test {
    IERC20[] public _tokens;

    function setUp() public {
        for (uint256 i = 0; i <= 150; ++i) {
            _tokens.push(IERC20(address(bytes20(bytes32(generatePrivateKey())))));
        }
    }

    function test_search(uint256 i, uint256 j) public {
        i = bound(i, 0, 150);
        j = bound(j, 0, 150);
        vm.assume(i != j);

        IERC20 i_token = _tokens[i];
        IERC20 j_token = _tokens[j];

        (uint256 found_i, uint256 found_j) = _getIJ(_tokens, i_token, j_token);

        assert(found_i == i);
        assert(found_j == j);
    }

    function test_best_case() public {
        IERC20 i_token = _tokens[0];
        IERC20 j_token = _tokens[1];

        (uint256 found_i, uint256 found_j) = _getIJ(_tokens, i_token, j_token);

        assert(found_i == 0);
        assert(found_j == 1);
    }

    function test_worst_case() public {
        IERC20 i_token = _tokens[149];
        IERC20 j_token = _tokens[150];

        (uint256 found_i, uint256 found_j) = _getIJ(_tokens, i_token, j_token);

        assert(found_i == 149);
        assert(found_j == 150);
    }

    function generatePrivateKey() public returns (bytes memory) {
        bytes memory privKeyData;

        string[] memory inputs = new string[](3);
        inputs[0] = "cast";
        inputs[1] = "wallet";
        inputs[2] = "new";

        // execute `cast wallet new` and return the output to res
        bytes memory res = vm.ffi(inputs);

        // cast private key is returned in last 64 bytes at the end of res
        privKeyData = slice(res, res.length - 64, 64);

        // convert the weird ASCII-byte values returned by cast call into a valid bytes32 value
        return privKeyData;
    }

    function slice(
        bytes memory _bytes,
        uint256 _start,
        uint256 _length
    )
        internal
        pure
        returns (bytes memory)
    {
        require(_length + 31 >= _length, "slice_overflow");
        require(_bytes.length >= _start + _length, "slice_outOfBounds");

        bytes memory tempBytes;

        assembly {
            switch iszero(_length)
            case 0 {
                // Get a location of some free memory and store it in tempBytes as
                // Solidity does for memory variables.
                tempBytes := mload(0x40)

                // The first word of the slice result is potentially a partial
                // word read from the original array. To read it, we calculate
                // the length of that partial word and start copying that many
                // bytes into the array. The first word we copy will start with
                // data we don't care about, but the last `lengthmod` bytes will
                // land at the beginning of the contents of the new array. When
                // we're done copying, we overwrite the full first word with
                // the actual length of the slice.
                let lengthmod := and(_length, 31)

                // The multiplication in the next line is necessary
                // because when slicing multiples of 32 bytes (lengthmod == 0)
                // the following copy loop was copying the origin's length
                // and then ending prematurely not copying everything it should.
                let mc := add(add(tempBytes, lengthmod), mul(0x20, iszero(lengthmod)))
                let end := add(mc, _length)

                for {
                    // The multiplication in the next line has the same exact purpose
                    // as the one above.
                    let cc := add(add(add(_bytes, lengthmod), mul(0x20, iszero(lengthmod))), _start)
                } lt(mc, end) {
                    mc := add(mc, 0x20)
                    cc := add(cc, 0x20)
                } {
                    mstore(mc, mload(cc))
                }

                mstore(tempBytes, _length)

                //update free-memory pointer
                //allocating the array padded to 32 bytes like the compiler does now
                mstore(0x40, and(add(mc, 31), not(31)))
            }
            //if we want a zero-length slice let's just return a zero-length array
            default {
                tempBytes := mload(0x40)
                //zero out the 32 bytes slice we are about to return
                //we need to do it because Solidity does not garbage collect
                mstore(tempBytes, 0)

                mstore(0x40, add(tempBytes, 0x20))
            }
        }

        return tempBytes;
    }
}
```

</details>

Don't write state changes until slippage has been checked for all tokens

• src\Well.sol

Check the slippage of each token before transferring, as the transfer causes a state change, and a subsequent revert will revert the changes to the initial state. By refactoring the code, you can save ≈30000+ gas per each token before the token that triggers the revert.

Code difference

diff --git a/src/Well.sol b/src/Well.sol
index 64dd3cc..ef6f9fc 100644
--- a/src/Well.sol
+++ b/src/Well.sol
@@ -474,6 +474,8 @@ contract Well is ERC20PermitUpgradeable, IWell, IWellErrors, ReentrancyGuardUpgr
             if (tokenAmountsOut[i] < minTokenAmountsOut[i]) {
                 revert SlippageOut(tokenAmountsOut[i], minTokenAmountsOut[i]);
             }
+        }
+        for (uint256 i; i < _tokens.length; ++i) {
             _tokens[i].safeTransfer(recipient, tokenAmountsOut[i]);
             reserves[i] = reserves[i] - tokenAmountsOut[i];
         }

Command: forge test --gas-report --ffi --match-path "test/Well.Splippage.t.sol"

Deployment gas cost increased by 5 808

Average functions call gas cost reduced by 11 287

Maximum functions call gas cost increased by 253

<details> <summary>OLD code perfomance:</summary>

```
[PASS] test_removeLiquidity() (gas: 175816)
[PASS] test_removeLiquidity_revertIf_SlippageOut_first() (gas: 53940)
[PASS] test_removeLiquidity_revertIf_SlippageOut_second() (gas: 88262)

| src/Well.sol:Well contract |                 |       |        |       |         |
| -------------------------- | --------------- | ----- | ------ | ----- | ------- |
| Deployment Cost            | Deployment Size |       |        |       |         |
| 4036006                    | 20189           |       |        |       |         |
| Function Name              | min             | avg   | median | max   | # calls |
| ...                        | ...             | ...   | ...    | ...   | ...     |
| removeLiquidity            | 33567           | 62846 | 67911  | 87060 | 3       |
```

</details> <details> <summary>NEW code perfomance:</summary>

```
[PASS] test_removeLiquidity() (gas: 176069)
[PASS] test_removeLiquidity_revertIf_SlippageOut_first() (gas: 53940)
[PASS] test_removeLiquidity_revertIf_SlippageOut_second() (gas: 54150)
| src/Well.sol:Well contract |                 |       |        |       |         |
| -------------------------- | --------------- | ----- | ------ | ----- | ------- |
| Deployment Cost            | Deployment Size |       |        |       |         |
| 4041814                    | 20218           |       |        |       |         |
| Function Name              | min             | avg   | median | max   | # calls |
| ...                        | ...             | ...   | ...    | ...   | ...     |
| removeLiquidity            | 33567           | 51559 | 33799  | 87313 | 3       |
```

</details> <details> <summary>test/Well.Splippage.t.sol</summary>

```solidity
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;

import {TestHelper, ConstantProduct2, IERC20, Balances} from "test/TestHelper.sol";
import {Snapshot, AddLiquidityAction, RemoveLiquidityAction, LiquidityHelper} from "test/LiquidityHelper.sol";
import {IWell} from "src/interfaces/IWell.sol";
import {IWellErrors} from "src/interfaces/IWellErrors.sol";

contract WellSlippageOutTest is LiquidityHelper {
    ConstantProduct2 cp;
    bytes constant data = "";
    uint256 constant addedLiquidity = 1000 * 1e18;

    function setUp() public {
        cp = new ConstantProduct2();
        setupWell(2);

        // Add liquidity. `user` now has (2 * 1000 * 1e27) LP tokens
        addLiquidityEqualAmount(user, addedLiquidity);
    }

    /// @dev removeLiquidity: remove to equal amounts of underlying
    function test_removeLiquidity() public prank(user) {
        uint256 lpAmountIn = 1000 * 1e24;

        uint256[] memory amountsOut = new uint256[](2);
        amountsOut[0] = 1000 * 1e18;
        amountsOut[1] = 1000 * 1e18;

        Snapshot memory before;
        RemoveLiquidityAction memory action;

        action.amounts = amountsOut;
        action.lpAmountIn = lpAmountIn;
        action.recipient = user;
        action.fees = new uint256[](2);

        (before, action) = beforeRemoveLiquidity(action);
        well.removeLiquidity(lpAmountIn, amountsOut, user, type(uint256).max);
        afterRemoveLiquidity(before, action);
        checkInvariant(address(well));
    }

    /// @dev removeLiquidity: reverts when user tries to remove too much of an underlying token
    function test_removeLiquidity_revertIf_SlippageOut_first() public prank(user) {
        uint256 lpAmountIn = 1000 * 1e24;

        uint256[] memory minTokenAmountsOut = new uint256[](2);
        minTokenAmountsOut[0] = 1001 * 1e18; // too high
        minTokenAmountsOut[1] = 1000 * 1e18;

        vm.expectRevert(abi.encodeWithSelector(IWellErrors.SlippageOut.selector, 1000 * 1e18, minTokenAmountsOut[0]));
        well.removeLiquidity(lpAmountIn, minTokenAmountsOut, user, type(uint256).max);
    }

    function test_removeLiquidity_revertIf_SlippageOut_second() public prank(user) {
        uint256 lpAmountIn = 1000 * 1e24;

        uint256[] memory minTokenAmountsOut = new uint256[](2);
        minTokenAmountsOut[0] = 1000 * 1e18;
        minTokenAmountsOut[1] = 1001 * 1e18; // too high

        vm.expectRevert(abi.encodeWithSelector(IWellErrors.SlippageOut.selector, 1000 * 1e18, minTokenAmountsOut[1]));
        well.removeLiquidity(lpAmountIn, minTokenAmountsOut, user, type(uint256).max);
    }

}
```

</details>

In removeLiquidityImbalanced check slippage before transfering tokens

• src\Well.sol

NOTE: _calcLpTokenSupply should not use token balances, which is not provided by the interface anyway, but the user can do it manually.

Code difference

diff --git a/src/Well.sol b/src/Well.sol
index 64dd3cc..f71371e 100644
--- a/src/Well.sol
+++ b/src/Well.sol
@@ -555,7 +555,6 @@ contract Well is ERC20PermitUpgradeable, IWell, IWellErrors, ReentrancyGuardUpgr
         uint256[] memory reserves = _updatePumps(_tokens.length);

         for (uint256 i; i < _tokens.length; ++i) {
-            _tokens[i].safeTransfer(recipient, tokenAmountsOut[i]);
             reserves[i] = reserves[i] - tokenAmountsOut[i];
         }

@@ -563,6 +562,9 @@ contract Well is ERC20PermitUpgradeable, IWell, IWellErrors, ReentrancyGuardUpgr
         if (lpAmountIn > maxLpAmountIn) {
             revert SlippageIn(lpAmountIn, maxLpAmountIn);
         }
+        for (uint256 i; i < _tokens.length; ++i) {
+            _tokens[i].safeTransfer(recipient, tokenAmountsOut[i]);
+        }
         _burn(msg.sender, lpAmountIn);

         _setReserves(_tokens, reserves);

forge test --gas-report --ffi --match-path "test/Well.SlippageIn.t.sol"

Deployment gas cost increased by 5 808

Minimum functions call gas cost reduced by 67 736

Average functions call gas cost reduced by 33 742

Maximum functions call gas cost increased by 253

<details> <summary>OLD code perfomance:</summary>

```
[PASS] test_removeLiquidityImbalanced() (gas: 172798)
[PASS] test_removeLiquidityImbalanced_revertIf_notEnoughLP() (gas: 118619)
| src/Well.sol:Well contract |                 |       |        |       |         |
| -------------------------- | --------------- | ----- | ------ | ----- | ------- |
| Deployment Cost            | Deployment Size |       |        |       |         |
| 4036006                    | 20189           |       |        |       |         |
| Function Name              | min             | avg   | median | max   | # calls |
| ...                        | ...             | ...   | ...    | ...   | ...     |
| removeLiquidityImbalanced  | 95369           | 96679 | 96679  | 97989 | 2       |
```

</details> <details> <summary>NEW code perfomance:</summary>

```
[PASS] test_removeLiquidityImbalanced() (gas: 173051)
[PASS] test_removeLiquidityImbalanced_revertIf_notEnoughLP() (gas: 50883)
| src/Well.sol:Well contract |                 |       |        |       |         |
| -------------------------- | --------------- | ----- | ------ | ----- | ------- |
| Deployment Cost            | Deployment Size |       |        |       |         |
| 4041814                    | 20218           |       |        |       |         |
| Function Name              | min             | avg   | median | max   | # calls |
| ...                        | ...             | ...   | ...    | ...   | ...     |
| removeLiquidityImbalanced  | 27633           | 62937 | 62937  | 98242 | 2       |
```

</details> <details> <summary>test/Well.SlippageIn.t.sol</summary>

```solidity
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;

import {TestHelper, ConstantProduct2, Balances} from "test/TestHelper.sol";
import {IWell} from "src/interfaces/IWell.sol";
import {IWellErrors} from "src/interfaces/IWellErrors.sol";

contract WellRemoveLiquidityImbalancedTest is TestHelper {
    event RemoveLiquidity(uint256 lpAmountIn, uint256[] tokenAmountsOut, address recipient);

    uint256[] tokenAmountsOut;
    uint256 requiredLpAmountIn;

    // Setup
    ConstantProduct2 cp;
    uint256 constant addedLiquidity = 1000 * 1e18;

    function setUp() public {
        cp = new ConstantProduct2();
        setupWell(2);

        // Add liquidity. `user` now has (2 * 1000 * 1e27) LP tokens
        addLiquidityEqualAmount(user, addedLiquidity);

        // Shared removal amounts
        tokenAmountsOut.push(500 * 1e18); // 500   token0
        tokenAmountsOut.push(506 * 1e17); //  50.6 token1
        requiredLpAmountIn = 290 * 1e24; // LP needed to remove `tokenAmountsOut`
    }

    /// @dev Base case
    function test_removeLiquidityImbalanced() public prank(user) {
        Balances memory userBalanceBefore = getBalances(user, well);

        uint256 initialLpAmount = userBalanceBefore.lp;
        uint256 maxLpAmountIn = requiredLpAmountIn;

        vm.expectEmit(true, true, true, true);
        emit RemoveLiquidity(maxLpAmountIn, tokenAmountsOut, user);
        well.removeLiquidityImbalanced(maxLpAmountIn, tokenAmountsOut, user, type(uint256).max);

        Balances memory userBalanceAfter = getBalances(user, well);
        Balances memory wellBalanceAfter = getBalances(address(well), well);

        // `user` balance of LP tokens decreases
        assertEq(userBalanceAfter.lp, initialLpAmount - maxLpAmountIn);

        // `user` balance of underlying tokens increases
        // assumes initial balance of zero
        assertEq(userBalanceAfter.tokens[0], tokenAmountsOut[0], "Incorrect token0 user balance");
        assertEq(userBalanceAfter.tokens[1], tokenAmountsOut[1], "Incorrect token1 user balance");

        // Well's reserve of underlying tokens decreases
        assertEq(wellBalanceAfter.tokens[0], 1500 * 1e18, "Incorrect token0 well reserve");
        assertEq(wellBalanceAfter.tokens[1], 19_494 * 1e17, "Incorrect token1 well reserve");
        checkInvariant(address(well));
    }

    function test_removeLiquidityImbalanced_revertIf_notEnoughLP() public prank(user) {
        uint256 maxLpAmountIn = 5 * 1e24;
        vm.expectRevert(abi.encodeWithSelector(IWellErrors.SlippageIn.selector, requiredLpAmountIn, maxLpAmountIn));
        well.removeLiquidityImbalanced(maxLpAmountIn, tokenAmountsOut, user, type(uint256).max);
    }
}
```

</details>

MultiFlowPump check for reserves != 0 twice

• src\pumps\MultiFlowPump.sol

Code difference

diff --git a/src/pumps/MultiFlowPump.sol b/src/pumps/MultiFlowPump.sol
index e2aab5d..a3ee999 100644
--- a/src/pumps/MultiFlowPump.sol
+++ b/src/pumps/MultiFlowPump.sol
@@ -81,10 +81,6 @@ contract MultiFlowPump is IPump, IMultiFlowPumpErrors, IInstantaneousPump, ICumu

         // If the last timestamp is 0, then the pump has never been used before.
         if (pumpState.lastTimestamp == 0) {
-            for (uint256 i; i < numberOfReserves; ++i) {
-                // If a reserve is 0, then the pump cannot be initialized.
-                if (reserves[i] == 0) return;
-            }
             _init(slot, uint40(block.timestamp), reserves);
             return;
         }

forge test --gas-report --ffi --match-path "test/pumps/Pump.Update.Init.t.sol"

Deployment gas cost reduced by 13 816

Minimum functions call gas cost increased by 356

Average functions call gas cost reduced by 15

Maximum functions call gas cost reduced by 386

<details> <summary>OLD code perfomance:</summary>

```
[PASS] test_first_update_init() (gas: 195951)
| src/pumps/MultiFlowPump.sol:MultiFlowPump contract |                 |       |        |       |         |
|----------------------------------------------------|-----------------|-------|--------|-------|---------|
| Deployment Cost                                    | Deployment Size |       |        |       |         |
| 3325658                                            | 18861           |       |        |       |         |
| Function Name                                      | min             | avg   | median | max   | # calls |
| update                                             | 3157            | 40385 | 40385  | 77614 | 2       |
```

</details> <details> <summary>NEW code perfomance:</summary>

```
[PASS] test_first_update_init() (gas: 195921)
| src/pumps/MultiFlowPump.sol:MultiFlowPump contract |                 |       |        |       |         |
|----------------------------------------------------|-----------------|-------|--------|-------|---------|
| Deployment Cost                                    | Deployment Size |       |        |       |         |
| 3311842                                            | 18792           |       |        |       |         |
| Function Name                                      | min             | avg   | median | max   | # calls |
| update                                             | 3513            | 40370 | 40370  | 77228 | 2       |
```

</details> <details> <summary>test/pumps/Pump.Update.Init.t.sol</summary>

```solidity
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;

import {console, TestHelper} from "test/TestHelper.sol";
import {MultiFlowPump, ABDKMathQuad} from "src/pumps/MultiFlowPump.sol";
import {from18, to18} from "test/pumps/PumpHelpers.sol";
import {MockReserveWell} from "mocks/wells/MockReserveWell.sol";
import {IMultiFlowPumpErrors} from "src/interfaces/pumps/IMultiFlowPumpErrors.sol";

import {log2, powu, UD60x18, wrap, unwrap} from "prb/math/UD60x18.sol";
import {exp2, log2, powu, UD60x18, wrap, unwrap, uUNIT} from "prb/math/UD60x18.sol";

contract PumpUpdateTest is TestHelper {
    using ABDKMathQuad for bytes16;

    MultiFlowPump pump;
    MockReserveWell mWell;
    uint256[] b = new uint256[](2);

    uint256 constant BLOCK_TIME = 12;

    /// @dev for this test, `user` = a Well that's calling the Pump
    function setUp() public {
        mWell = new MockReserveWell();
        initUser();
        pump = new MultiFlowPump(
            from18(0.5e18), // cap reserves if changed +/- 50% per block
            from18(0.5e18), // cap reserves if changed +/- 50% per block
            12, // block time
            from18(0.9e18) // ema alpha
        );
    }

    function test_first_update_init() public {
        // Send first update to the Pump, which will initialize it
        vm.prank(user);
        b[0] = 1e6;
        b[1] = 2e6;
        mWell.update(address(pump), b, new bytes(0));
        mWell.update(address(pump), b, new bytes(0));
    }
}

```

</details>