Timeswap contest - mookimgo's results

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-token/src/TimeswapV2LiquidityToken.sol#L114 https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-token/src/TimeswapV2Token.sol#L103

Vulnerability details

Impact

Assuming ERC1155Enumerable is acting normally, there is a Accounting Issue about TimeswapV2LiquidityToken and TimeswapV2Token's tokenId.

Different liquidities can have the same tokenId, leading to serious balance manipulation.

I'm submitting this issue as medium because current implementation ERC1155Enumerable is wrong, which exactly mitigate this issue making it not exploitable. But this issue will become dangerous once we fixed ERC1155Enumerable.

Proof of Concept

In this PoC, the attacker will do these steps:

1. Add liquidity of token0 and token1, thus receive TimeswapV2LiquidityToken tokenId 1.
2. Add liquidity of token2 and token3, thus receive TimeswapV2LiquidityToken tokenId 2.
3. Burn his liquidity from step1, which will make totalSupply decrease (if ERC1155Enumerable has been patched)
4. Add liquidity of token4 and token5, and receive TimeswapV2LiquidityToken tokenId 2. This is wrong tokenId, which should be 3.

Explanation:

https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-token/src/TimeswapV2LiquidityToken.sol#L112

As the comment said if the position does not exist, create it, but the new tokenId is set as totalSupply() + 1

Function totalSupply is defined in packages/v2-token/src/base/ERC1155Enumerable.sol, which is simply _allTokens.length: https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-token/src/base/ERC1155Enumerable.sol#L37-L38

_allTokens.length can be decreased in _removeTokenFromAllTokensEnumeration function, which is called by _removeTokenEnumeration, and by _afterTokenTransfer. In simple words, when all token amounts for a specific tokenId are burned (_idTotalSupply[id] == 0), totalSupply should be decreased.

Current implementation of ERC1155Enumerable has a bug, which will never trigger _removeTokenFromAllTokensEnumeration: Calling _removeTokenEnumeration needs amount>0, but only _idTotalSupply[id] == 0 can trigger _removeTokenFromAllTokensEnumeration.

    function _removeTokenEnumeration(address from, address to, uint256 id, uint256 amount) internal {
        if (to == address(0)) {
            if (_idTotalSupply[id] == 0 && _additionalConditionRemoveTokenFromAllTokensEnumeration(id)) _removeTokenFromAllTokensEnumeration(id);
            _idTotalSupply[id] -= amount;
        }

Once the above code get fixed(swapping the if line and _idTotalSupply[id] -= amount; line, patch given below), this issue becomes exploitable, making the accounting of LP wrong.

---

PoC steps:

First, we need to patch two contract:

• making TimeswapV2LiquidityToken's _timeswapV2LiquidityTokenPositionIds as public for testing, this can be removed when depolying
• ERC1155Enumerable's _removeTokenEnumeration has been patched to behave correctly, which will decrease totalSupply when all token amount of a specific tokenId has been burned.

diff --git a/packages/v2-token/src/TimeswapV2LiquidityToken.sol b/packages/v2-token/src/TimeswapV2LiquidityToken.sol
index 2f71a25..f3910d9 100644
--- a/packages/v2-token/src/TimeswapV2LiquidityToken.sol
+++ b/packages/v2-token/src/TimeswapV2LiquidityToken.sol
@@ -42,7 +42,7 @@ contract TimeswapV2LiquidityToken is ITimeswapV2LiquidityToken, ERC1155Enumerabl
 
     mapping(uint256 => TimeswapV2LiquidityTokenPosition) private _timeswapV2LiquidityTokenPositions;
 
-    mapping(bytes32 => uint256) private _timeswapV2LiquidityTokenPositionIds;
+    mapping(bytes32 => uint256) public _timeswapV2LiquidityTokenPositionIds;
 
     mapping(uint256 => mapping(address => FeesPosition)) private _feesPositions;
 
diff --git a/packages/v2-token/src/base/ERC1155Enumerable.sol b/packages/v2-token/src/base/ERC1155Enumerable.sol
index 4ec23ff..4f51fb4 100644
--- a/packages/v2-token/src/base/ERC1155Enumerable.sol
+++ b/packages/v2-token/src/base/ERC1155Enumerable.sol
@@ -91,8 +91,8 @@ abstract contract ERC1155Enumerable is IERC1155Enumerable, ERC1155 {
     /// @dev Remove token enumeration list if necessary.
     function _removeTokenEnumeration(address from, address to, uint256 id, uint256 amount) internal {
         if (to == address(0)) {
-            if (_idTotalSupply[id] == 0 && _additionalConditionRemoveTokenFromAllTokensEnumeration(id)) _removeTokenFromAllTokensEnumeration(id);
             _idTotalSupply[id] -= amount;
+            if (_idTotalSupply[id] == 0 && _additionalConditionRemoveTokenFromAllTokensEnumeration(id)) _removeTokenFromAllTokensEnumeration(id);
         }
 
         if (from != address(0) && from != to) {

Add a new test file in 2023-01-timeswap/packages/v2-token/test/TimeswapV2LiquidityToken_MultiMint.t.sol:

// SPDX-License-Identifier: UNLICENSED
pragma solidity =0.8.8;

import "forge-std/Test.sol";

import "forge-std/console.sol";

import "../src/TimeswapV2LiquidityToken.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@timeswap-labs/v2-option/src/TimeswapV2OptionFactory.sol";
import "@timeswap-labs/v2-option/src/interfaces/ITimeswapV2Option.sol";
import {TimeswapV2LiquidityTokenCollectParam} from "../src/structs/Param.sol";
import "@timeswap-labs/v2-pool/src/TimeswapV2PoolFactory.sol";
import "@timeswap-labs/v2-pool/src/interfaces/ITimeswapV2Pool.sol";
import {TimeswapV2PoolMintParam} from "@timeswap-labs/v2-pool/src/structs/Param.sol";
import {TimeswapV2PoolMintChoiceCallbackParam, TimeswapV2PoolMintCallbackParam} from "@timeswap-labs/v2-pool/src/structs/CallbackParam.sol";

import {TimeswapV2OptionMintCallbackParam, TimeswapV2OptionSwapCallbackParam} from "@timeswap-labs/v2-option/src/structs/CallbackParam.sol";

// import "@timeswap-labs/v2-option/src/TimeswapV2OptionFactory.sol";
// // import "@timeswap-labs/v2-option/src/interfaces/ITimeswapV2Option.sol";
import "@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol";
import {TimeswapV2LiquidityTokenPosition, PositionLibrary} from "../src/structs/Position.sol";
import {TimeswapV2PoolMint} from "@timeswap-labs/v2-pool/src/enums/Transaction.sol";
import {TimeswapV2OptionMint} from "@timeswap-labs/v2-option/src/enums/Transaction.sol";

import {StrikeConversion} from "@timeswap-labs/v2-library/src/StrikeConversion.sol";
import {DurationCalculation} from "@timeswap-labs/v2-pool/src/libraries/DurationCalculation.sol";
import {FullMath} from "@timeswap-labs/v2-library/src/FullMath.sol";

contract HelperERC20 is ERC20 {
    constructor(string memory _name, string memory _symbol) ERC20(_name, _symbol) {
        _mint(msg.sender, type(uint256).max);
    }
}
struct Timestamps {
    uint256 maturity;
    uint256 timeNow;
}
struct MintOutput {
    uint160 liquidityAmount;
    uint256 long0Amount;
    uint256 long1Amount;
    uint256 shortAmount;
    bytes data;
}

contract TimeswapV2LiquidityTokenTest is Test, ERC1155Holder {
    ITimeswapV2Option opPair;
    ITimeswapV2Option opPair2;
    ITimeswapV2Option opPair3;
    ITimeswapV2Option opPairCurrent;
    TimeswapV2OptionFactory optionFactory;
    TimeswapV2PoolFactory poolFactory;
    ITimeswapV2Pool pool;
    ITimeswapV2Pool pool2;
    ITimeswapV2Pool pool3;
    ITimeswapV2Pool poolCurrent;
    using PositionLibrary for TimeswapV2LiquidityTokenPosition;

    uint256 chosenTransactionFee = 5;
    uint256 chosenProtocolFee = 4;

    HelperERC20 token0;
    HelperERC20 token1;
    HelperERC20 token2;
    HelperERC20 token3;
    HelperERC20 token4;
    HelperERC20 token5;
    HelperERC20 token0Current;
    HelperERC20 token1Current;
    TimeswapV2LiquidityToken mockLiquidityToken;

    function timeswapV2PoolMintChoiceCallback(TimeswapV2PoolMintChoiceCallbackParam calldata param) external returns (uint256 long0Amount, uint256 long1Amount, bytes memory data) {
        vm.assume(param.longAmount < (1 << 127));
        long0Amount = StrikeConversion.turn(param.longAmount / 2, param.strike, false, true) + 1;
        long1Amount = StrikeConversion.turn(param.longAmount / 2, param.strike, true, true) + 1;
        vm.assume(
            param.longAmount < StrikeConversion.combine(long0Amount, long1Amount, param.strike, false) && param.shortAmount < StrikeConversion.combine(long0Amount, long1Amount, param.strike, false)
        );
    }

    function timeswapV2PoolMintCallback(TimeswapV2PoolMintCallbackParam calldata param) external returns (bytes memory data) {
        // have to transfer param.long0Amount, param.long1Amount and param.short to msg.sender
        console.log(param.long0Amount, param.long1Amount);
        TimeswapV2OptionMintParam memory mparam = TimeswapV2OptionMintParam({
            strike: param.strike,
            maturity: param.maturity,
            long0To: msg.sender,
            long1To: msg.sender,
            shortTo: msg.sender,
            transaction: TimeswapV2OptionMint.GivenTokensAndLongs,
            amount0: param.long0Amount,
            amount1: param.long1Amount,
            data: ""
        });
        opPairCurrent.mint(mparam);
        console.log("opPair mint ok");
    }

    function timeswapV2OptionMintCallback(TimeswapV2OptionMintCallbackParam calldata param) external returns (bytes memory data) {
        data = param.data;
        //console.log("token0 bal:", token0.balanceOf(address(this)));
        //console.log("token1 bal:", token1.balanceOf(address(this)));
        token0Current.transfer(msg.sender, param.token0AndLong0Amount);
        token1Current.transfer(msg.sender, param.token1AndLong1Amount);
    }

    function timeswapV2LiquidityTokenMintCallback(TimeswapV2LiquidityTokenMintCallbackParam calldata param) external returns (bytes memory data) {
        TimeswapV2PoolMintParam memory param1 = TimeswapV2PoolMintParam({
            strike: param.strike, 
            maturity: param.maturity, 
            to: address(this), 
            transaction: TimeswapV2PoolMint.GivenLiquidity, 
            delta: param.liquidityAmount, 
            data: ""}
        );

        poolCurrent.mint(param1);
        poolCurrent.transferLiquidity(param.strike, param.maturity, msg.sender, param.liquidityAmount);
        data = bytes("");
    }

    function setUp() public {
        optionFactory = new TimeswapV2OptionFactory();
        token0 = new HelperERC20("Token A", "A");
        token1 = new HelperERC20("Token B", "B");
        token2 = new HelperERC20("Token C", "C");
        token3 = new HelperERC20("Token D", "D");
        token4 = new HelperERC20("Token E", "E");
        token5 = new HelperERC20("Token F", "F");
        if (address(token1) < address(token0)) {
            (token0, token1) = (token1, token0);
        }
        if (address(token3) < address(token2)) {
            (token2, token3) = (token3, token2);
        }
        if (address(token5) < address(token4)) {
            (token4, token5) = (token5, token4);
        }
        address opAddress = optionFactory.create(address(token0), address(token1));
        opPair = ITimeswapV2Option(opAddress);
        address opAddress2 = optionFactory.create(address(token2), address(token3));
        opPair2 = ITimeswapV2Option(opAddress2);
        address opAddress3 = optionFactory.create(address(token4), address(token5));
        opPair3 = ITimeswapV2Option(opAddress3);
        poolFactory = new TimeswapV2PoolFactory(address(this), chosenTransactionFee, chosenProtocolFee);
        pool = ITimeswapV2Pool(poolFactory.create(opAddress));
        pool2 = ITimeswapV2Pool(poolFactory.create(opAddress2));
        pool3 = ITimeswapV2Pool(poolFactory.create(opAddress3));
        mockLiquidityToken = new TimeswapV2LiquidityToken(address(optionFactory), address(poolFactory));
    }

    function testMint(uint256 strike, uint160 amt, uint256 maturity, uint160 rate, address to) public {
        setUp();

        // vm.assume(strike != 0 && (maturity < type(uint96).max) && (maturity > 10000) && amt > 100 && delta != 0 && rate != 0);
        vm.assume(to != address(0));
        vm.assume(
            maturity < type(uint96).max &&
                amt < type(uint160).max &&
                amt != 0 &&
                to != address(0) &&
                strike != 0 &&
                maturity > block.timestamp &&
                maturity > 10000 && rate>0
        );

        console.log("init");
        pool.initialize(strike, maturity, rate);
        pool2.initialize(strike, maturity, rate);
        pool3.initialize(strike, maturity, rate);

        //TimeswapV2PoolMintParam memory param = TimeswapV2PoolMintParam({strike: strike, maturity: maturity, to: address(this), transaction: TimeswapV2PoolMint.GivenLiquidity, delta: amt, data: ""});

        //MintOutput memory response;
        //(response.liquidityAmount, response.long0Amount, response.long1Amount, response.shortAmount, response.data) = pool.mint(param);
        uint256 id1;
        uint256 id2;
        {
            token0Current = token0;
            token1Current = token1;
            poolCurrent = pool;
            opPairCurrent = opPair;
            TimeswapV2LiquidityTokenMintParam memory liqTokenMintParam = TimeswapV2LiquidityTokenMintParam({
                token0: address(token0Current),
                token1: address(token1Current),
                strike: strike,
                maturity: maturity,
                to: address(this),
                liquidityAmount: amt,
                data: ""
            });

            mockLiquidityToken.mint(liqTokenMintParam);
            //console.log(mockLiquidityToken.balanceOf(address(this)));
            TimeswapV2LiquidityTokenPosition memory timeswapV2LiquidityTokenPosition = TimeswapV2LiquidityTokenPosition({
                token0: address(token0Current),
                token1: address(token1Current),
                strike: strike,
                maturity: maturity
            });

            bytes32 key1 = timeswapV2LiquidityTokenPosition.toKey();
            id1 = mockLiquidityToken._timeswapV2LiquidityTokenPositionIds(key1);
            console.log("key1:");
            console.logBytes32(key1);
            console.log("id1:", id1);
            assertEq(mockLiquidityToken.balanceOf(address(this), id1), amt);
            assertEq(mockLiquidityToken.totalSupply(), 1);
            //console.log("_idTotalSupply id1:", mockLiquidityToken._idTotalSupply(id1));
            console.log("========");
        }


        {
            token0Current = token2;
            token1Current = token3;
            poolCurrent = pool2;
            opPairCurrent = opPair2;
            TimeswapV2LiquidityTokenMintParam memory liqTokenMintParam2 = TimeswapV2LiquidityTokenMintParam({
                token0: address(token0Current),
                token1: address(token1Current),
                strike: strike,
                maturity: maturity,
                to: address(this),
                liquidityAmount: amt,
                data: ""
            });

            mockLiquidityToken.mint(liqTokenMintParam2);
            //console.log(mockLiquidityToken.balanceOf(address(this)));
            TimeswapV2LiquidityTokenPosition memory timeswapV2LiquidityTokenPosition2 = TimeswapV2LiquidityTokenPosition({
                token0: address(token0Current),
                token1: address(token1Current),
                strike: strike,
                maturity: maturity
            });

            bytes32 key2 = timeswapV2LiquidityTokenPosition2.toKey();
            id2 = mockLiquidityToken._timeswapV2LiquidityTokenPositionIds(key2);
            console.log("key2:");
            console.logBytes32(key2);
            console.log("id2:", id2);
            assertEq(mockLiquidityToken.balanceOf(address(this), id2), amt);
            assertEq(mockLiquidityToken.totalSupply(), 2);
            console.log("========");
        }

        TimeswapV2LiquidityTokenBurnParam memory burnParam = TimeswapV2LiquidityTokenBurnParam({
            token0: address(token0),
            token1: address(token1),
            strike: strike,
            maturity: maturity,
            to: address(this),
            liquidityAmount: amt,
            data: ""
        });
        mockLiquidityToken.burn(burnParam);
        console.log("balanceOf id1:", mockLiquidityToken.balanceOf(address(this), id1));
        //console.log("_idTotalSupply id1:", mockLiquidityToken._idTotalSupply(id1));
        console.log("current totalSupply():", mockLiquidityToken.totalSupply());

        {
            token0Current = token4;
            token1Current = token5;
            poolCurrent = pool3;
            opPairCurrent = opPair3;
            TimeswapV2LiquidityTokenMintParam memory liqTokenMintParam3 = TimeswapV2LiquidityTokenMintParam({
                token0: address(token0Current),
                token1: address(token1Current),
                strike: strike,
                maturity: maturity,
                to: address(this),
                liquidityAmount: amt,
                data: ""
            });

            mockLiquidityToken.mint(liqTokenMintParam3);
            //console.log(mockLiquidityToken.balanceOf(address(this)));
            TimeswapV2LiquidityTokenPosition memory timeswapV2LiquidityTokenPosition3 = TimeswapV2LiquidityTokenPosition({
                token0: address(token0Current),
                token1: address(token1Current),
                strike: strike,
                maturity: maturity
            });

            bytes32 key3 = timeswapV2LiquidityTokenPosition3.toKey();
            uint256 id3 = mockLiquidityToken._timeswapV2LiquidityTokenPositionIds(key3);
            console.log("key3:");
            console.logBytes32(key3);
            console.log("id3:", id3);
            //assertEq(mockLiquidityToken.balanceOf(address(this), id3), amt);
            if (id2 == id3) {revert("id3 should not equal to id2");}
            console.log("========");
        }

        console.log("yo");
    }
}

Here is the log for the above test: forge test --match-path test/TimeswapV2LiquidityToken_MultiMint.t.sol -vv

Running 1 test for test/TimeswapV2LiquidityToken.t.sol:TimeswapV2LiquidityTokenTest
[FAIL. Reason: id3 should not equal to id2 Counterexample: calldata=0x31b83c070000000000000000000000000000000000000000000000000000000000000d77000000000000000000000000000000000000000000000000000000000000234100000000000000000000000000000000000000000000000000000000277c306f00000000000000000000000000000000000000000000000000000000000032e0000000000000000000000000000000000000000000000000000000000000025f, args=[3447, 9025, 662450287, 13024, 0x000000000000000000000000000000000000025F]] testMint(uint256,uint160,uint256,uint160,address) (runs: 0, μ: 0, ~: 0)
Logs:
  init
  2709883200956651719220887728062100075977988725238523898809710331 27450636006266724768954781627
  opPair mint ok
  key1:
  0x3ad1cfe6142808456d576d32877db082ef58ce80e40fb5019d9e5f73aebfde46
  id1: 1
  ========
  2709883200956651719220887728062100075977988725238523898809710331 27450636006266724768954781627
  opPair mint ok
  key2:
  0x4b911bdfb2c97775c28fae58288d53335ea7b59d3675acf0460ff4083897e18c
  id2: 2
  ========
  balanceOf id1: 0
  current totalSupply(): 1
  2709883200956651719220887728062100075977988725238523898809710331 27450636006266724768954781627
  opPair mint ok
  key3:
  0x6b43d3a16273d9e9f13739b825952b03e59127b9d41c4e0d9d58d635e8d2f5d2
  id3: 2

Test result: FAILED. 0 passed; 1 failed; finished in 91.76ms

Failing tests:
Encountered 1 failing test in test/TimeswapV2LiquidityToken.t.sol:TimeswapV2LiquidityTokenTest
[FAIL. Reason: id3 should not equal to id2 Counterexample: calldata=0x31b83c070000000000000000000000000000000000000000000000000000000000000d77000000000000000000000000000000000000000000000000000000000000234100000000000000000000000000000000000000000000000000000000277c306f00000000000000000000000000000000000000000000000000000000000032e0000000000000000000000000000000000000000000000000000000000000025f, args=[3447, 9025, 662450287, 13024, 0x000000000000000000000000000000000000025F]] testMint(uint256,uint160,uint256,uint160,address) (runs: 0, μ: 0, ~: 0)

Encountered a total of 1 failing tests, 0 tests succeeded

Tools Used

Manual code reading

Recommended Mitigation Steps

Do not use totalSupply() or other maybe-decreasing variables for new tokenId.

Patch file can be like this:

diff --git a/packages/v2-token/src/TimeswapV2LiquidityToken.sol b/packages/v2-token/src/TimeswapV2LiquidityToken.sol
index 2f71a25..94e4006 100644
--- a/packages/v2-token/src/TimeswapV2LiquidityToken.sol
+++ b/packages/v2-token/src/TimeswapV2LiquidityToken.sol
@@ -32,6 +32,7 @@ contract TimeswapV2LiquidityToken is ITimeswapV2LiquidityToken, ERC1155Enumerabl
 
     address public immutable optionFactory;
     address public immutable poolFactory;
+    uint256 public tokenIdCounter;
 
     constructor(address chosenOptionFactory, address chosenPoolFactory) ERC1155("Timeswap V2 uint160 address") {
         optionFactory = chosenOptionFactory;
@@ -111,7 +112,7 @@ contract TimeswapV2LiquidityToken is ITimeswapV2LiquidityToken, ERC1155Enumerabl
 
         // if the position does not exist, create it
         if (id == 0) {
-            id = totalSupply() + 1;
+            id = ++tokenIdCounter;
             _timeswapV2LiquidityTokenPositions[id] = timeswapV2LiquidityTokenPosition;
             _timeswapV2LiquidityTokenPositionIds[key] = id;
         }

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/7cb8c18ba6d0871033ddf518dd79c4444e2f89cc/packages/v2-token/src/base/ERC1155Enumerable.sol#L92-L96

Vulnerability details

Impact

_removeTokenFromAllTokensEnumeration can never be called, because of wrong order of if statement and _idTotalSupply[id] -= amount;. This leads to totalSupply do not correctly minus 1 when all token amounts for a specific tokenId get burned.

Proof of Concept

    function _afterTokenTransfer(address, address from, address to, uint256[] memory ids, uint256[] memory amounts, bytes memory) internal virtual override {
        for (uint256 i; i < ids.length; ) {
            if (amounts[i] != 0) _removeTokenEnumeration(from, to, ids[i], amounts[i]);

            unchecked {
                ++i;
            }
        }
    }

    /// @dev Remove token enumeration list if necessary.
    function _removeTokenEnumeration(address from, address to, uint256 id, uint256 amount) internal {
        if (to == address(0)) {
            if (_idTotalSupply[id] == 0 && _additionalConditionRemoveTokenFromAllTokensEnumeration(id)) _removeTokenFromAllTokensEnumeration(id);
            _idTotalSupply[id] -= amount;
        }

To call _removeTokenFromAllTokensEnumeration, which is the only place where _allTokens's length (aka totalSupply()) can be decreased, the only caller is _afterTokenTransfer, which require transfer amount>0.

However, to trigger _removeTokenFromAllTokensEnumeration, the _idTotalSupply[id] == 0 and to == address(0) must hold, which means it can only happens when we're burning token and there is already no tokens left before _idTotalSupply[id] -= amount;. The can not happen (we cannot burn 0 balance due to amount>0 check), so _removeTokenFromAllTokensEnumeration can never be triggered.

Tools Used

Manual code reading.

Recommended Mitigation Steps

Change the order:

    function _removeTokenEnumeration(address from, address to, uint256 id, uint256 amount) internal {
        if (to == address(0)) {
            _idTotalSupply[id] -= amount;
            if (_idTotalSupply[id] == 0 && _additionalConditionRemoveTokenFromAllTokensEnumeration(id)) _removeTokenFromAllTokensEnumeration(id);
        }

1. forge-std dependency should be removed from TimeswapV2Token

https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-token/src/TimeswapV2Token.sol#L109

console.log should be removed before deploying.

2. misleading docs of ReentrancyGuard

https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-pool/src/libraries/ReentrancyGuard.sol#L21

@notice Reverts when balanceTarget is not zero.

but there's no balanceTarget, this comment should be removed.

3. misleading numberOfPairs function in TimeswapV2OptionFactory and TimeswapV2PoolFactory

https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-option/src/TimeswapV2OptionFactory.sol#L36 https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-pool/src/TimeswapV2PoolFactory.sol#L52

numberOfPairs function return length of getByIndex, but the getByIndex list is not modified in create.

Remove this numberOfPairs function and getByIndex variable, or add a write getByIndex.push(optionPair) in create.